From edfce99771835bccbf18a06734ebe054a1bc4235 Mon Sep 17 00:00:00 2001 From: Yasss Date: Tue, 2 Apr 2019 16:38:39 +0200 Subject: [PATCH 1/7] Add GTE operator --- rule/expr.go | 76 +++++++++++++++++++++++++++++++++-- rule/expr_test.go | 100 +++++++++++++++++++++++++++++++++++++++++++++- rule/json.go | 4 ++ 3 files changed, 175 insertions(+), 5 deletions(-) diff --git a/rule/expr.go b/rule/expr.go index 0cf908c..471d3fb 100644 --- a/rule/expr.go +++ b/rule/expr.go @@ -279,6 +279,46 @@ func (n *exprGT) Eval(params Params) (*Value, error) { return BoolValue(true), nil } +type exprGTE struct { + operator +} + +// GTE creates an expression that takes at least two operands and +// evaluates to true if each successive operand has a greater or equal value +// compared to the next. +func GTE(v1, v2 Expr, vN ...Expr) Expr { + return &exprGTE{ + operator: operator{ + kind: "gte", + operands: append([]Expr{v1, v2}, vN...), + }, + } +} + +func (n *exprGTE) Eval(params Params) (*Value, error) { + if len(n.operands) < 2 { + return nil, errors.New("invalid number of operands in GTE func") + } + + vA, err := n.operands[0].Eval(params) + if err != nil { + return nil, err + } + + for i := 1; i < len(n.operands); i++ { + vB, err := n.operands[i].Eval(params) + if err != nil { + return nil, err + } + + if !vA.GTE(vB) { + return BoolValue(false), nil + } + } + + return BoolValue(true), nil +} + type exprFNV struct { operator } @@ -312,7 +352,6 @@ func (n *exprFNV) Eval(params Params) (*Value, error) { return Int64Value(int64(h32.Sum32())), nil } - type exprPercentile struct { operator } @@ -349,7 +388,6 @@ func (n *exprPercentile) Eval(params Params) (*Value, error) { return BoolValue(false), nil } - // Param is an expression used to select a parameter passed during evaluation and return its corresponding value. type Param struct { Kind string `json:"kind"` @@ -534,6 +572,39 @@ func (v *Value) GT(other *Value) bool { return false } +// GTE reports whether v is greater or equal than other. +func (v *Value) GTE(other *Value) bool { + switch v.Type { + case "bool": + v1, _ := strconv.ParseBool(v.Data) + v2, _ := strconv.ParseBool(other.Data) + if !v1 && v2 { + return false + } + return true + case "string": + if v.Data < other.Data { + return false + } + return true + case "int64": + v1, _ := strconv.ParseInt(v.Data, 10, 64) + v2, _ := strconv.ParseInt(other.Data, 10, 64) + if v1 < v2 { + return false + } + return true + case "float64": + v1, _ := strconv.ParseFloat(v.Data, 64) + v2, _ := strconv.ParseFloat(other.Data, 64) + if v1 < v2 { + return false + } + return true + } + return false +} + type operander interface { Operands() []Expr } @@ -557,7 +628,6 @@ func walk(expr Expr, fn func(Expr) error) error { return nil } - // exprToInt64 returns the go-native int64 value of an expression // evaluated with params. func exprToInt64(e Expr, params Params) (int64, error) { diff --git a/rule/expr_test.go b/rule/expr_test.go index 0684af3..a84db5e 100644 --- a/rule/expr_test.go +++ b/rule/expr_test.go @@ -195,7 +195,7 @@ func TestIn(t *testing.T) { }) } -func TestGt(t *testing.T) { +func TestGT(t *testing.T) { cases := []struct { name string m1 mockExpr @@ -268,6 +268,103 @@ func TestGt(t *testing.T) { } } +func TestGTE(t *testing.T) { + cases := []struct { + name string + m1 mockExpr + m2 mockExpr + expected *rule.Value + }{ + { + name: "String: true#1", + m1: mockExpr{val: rule.StringValue("abc")}, + m2: mockExpr{val: rule.StringValue("abc")}, + expected: rule.BoolValue(true), + }, + { + name: "String: true#2", + m1: mockExpr{val: rule.StringValue("abd")}, + m2: mockExpr{val: rule.StringValue("abc")}, + expected: rule.BoolValue(true), + }, + { + name: "String: false", + m1: mockExpr{val: rule.StringValue("abc")}, + m2: mockExpr{val: rule.StringValue("abd")}, + expected: rule.BoolValue(false), + }, + { + name: "Bool: true#1", + m1: mockExpr{val: rule.BoolValue(true)}, + m2: mockExpr{val: rule.BoolValue(false)}, + expected: rule.BoolValue(true), + }, + { + name: "Bool: true#2", + m1: mockExpr{val: rule.BoolValue(true)}, + m2: mockExpr{val: rule.BoolValue(true)}, + expected: rule.BoolValue(true), + }, + { + name: "Bool: true#3", + m1: mockExpr{val: rule.BoolValue(false)}, + m2: mockExpr{val: rule.BoolValue(false)}, + expected: rule.BoolValue(true), + }, + { + name: "Bool: false", + m1: mockExpr{val: rule.BoolValue(false)}, + m2: mockExpr{val: rule.BoolValue(true)}, + expected: rule.BoolValue(false), + }, + { + name: "Int64: true#1", + m1: mockExpr{val: rule.Int64Value(12)}, + m2: mockExpr{val: rule.Int64Value(11)}, + expected: rule.BoolValue(true), + }, + { + name: "Int64: true#2", + m1: mockExpr{val: rule.Int64Value(12)}, + m2: mockExpr{val: rule.Int64Value(12)}, + expected: rule.BoolValue(true), + }, + { + name: "Int64: false", + m1: mockExpr{val: rule.Int64Value(11)}, + m2: mockExpr{val: rule.Int64Value(12)}, + expected: rule.BoolValue(false), + }, + { + name: "Float64: true#1", + m1: mockExpr{val: rule.Float64Value(12.1)}, + m2: mockExpr{val: rule.Float64Value(12.0)}, + expected: rule.BoolValue(true), + }, + { + name: "Float64: true#2", + m1: mockExpr{val: rule.Float64Value(12.1)}, + m2: mockExpr{val: rule.Float64Value(12.1)}, + expected: rule.BoolValue(true), + }, + { + name: "Float64: false", + m1: mockExpr{val: rule.Float64Value(12.0)}, + m2: mockExpr{val: rule.Float64Value(12.1)}, + expected: rule.BoolValue(false), + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gte := rule.GTE(&tc.m1, &tc.m2) + val, err := gte.Eval(nil) + require.NoError(t, err) + require.Equal(t, tc.expected, val) + }) + } +} + func TestFNV(t *testing.T) { cases := []struct { name string @@ -328,7 +425,6 @@ func TestPercentile(t *testing.T) { require.Equal(t, rule.BoolValue(false), res) } - func TestParam(t *testing.T) { t.Run("OK", func(t *testing.T) { v := rule.StringParam("foo") diff --git a/rule/json.go b/rule/json.go index 6f86c32..334a097 100644 --- a/rule/json.go +++ b/rule/json.go @@ -109,6 +109,10 @@ func unmarshalExpr(kind string, data []byte) (Expr, error) { var gt exprGT e = > err = gt.UnmarshalJSON(data) + case "gte": + var gte exprGTE + e = >e + err = gte.UnmarshalJSON(data) case "percentile": var percentile exprPercentile e = &percentile From 27a6746b83af173d26fdbce9dbddc2b0a25d457f Mon Sep 17 00:00:00 2001 From: Yasss Date: Tue, 2 Apr 2019 16:48:24 +0200 Subject: [PATCH 2/7] Add LT operator --- rule/expr.go | 79 +++++++++++++++++++++++++++++++++++++++++++++++ rule/expr_test.go | 73 +++++++++++++++++++++++++++++++++++++++++++ rule/json.go | 4 +++ 3 files changed, 156 insertions(+) diff --git a/rule/expr.go b/rule/expr.go index 471d3fb..09f0800 100644 --- a/rule/expr.go +++ b/rule/expr.go @@ -334,6 +334,22 @@ func FNV(v Expr) Expr { } } +type exprLT struct { + operator +} + +// LT creates an expression that takes at least two operands and +// evaluates to true if each successive operand has a lower value +// compared to the next. +func LT(v1, v2 Expr, vN ...Expr) Expr { + return &exprLT{ + operator: operator{ + kind: "lt", + operands: append([]Expr{v1, v2}, vN...), + }, + } +} + func (n *exprFNV) Eval(params Params) (*Value, error) { if len(n.operands) != 1 { return nil, errors.New("invalid number of operands in FNV func") @@ -388,6 +404,30 @@ func (n *exprPercentile) Eval(params Params) (*Value, error) { return BoolValue(false), nil } +func (n *exprLT) Eval(params Params) (*Value, error) { + if len(n.operands) < 2 { + return nil, errors.New("invalid number of operands in LT func") + } + + vA, err := n.operands[0].Eval(params) + if err != nil { + return nil, err + } + + for i := 1; i < len(n.operands); i++ { + vB, err := n.operands[i].Eval(params) + if err != nil { + return nil, err + } + + if !vA.LT(vB) { + return BoolValue(false), nil + } + } + + return BoolValue(true), nil +} + // Param is an expression used to select a parameter passed during evaluation and return its corresponding value. type Param struct { Kind string `json:"kind"` @@ -605,6 +645,45 @@ func (v *Value) GTE(other *Value) bool { return false } +// LT reports whether v is less than other. +func (v *Value) LT(other *Value) bool { + switch v.Type { + case "bool": + v1, _ := strconv.ParseBool(v.Data) + if v1 { + // If v1 is True then it's not less than v2, and we can be done already. + return false + } + + v2, _ := strconv.ParseBool(other.Data) + if !v2 { + // If v2 is False then v1 can't be less than it.. + return false + } + return true + case "string": + if v.Data >= other.Data { + return false + } + return true + case "int64": + v1, _ := strconv.ParseInt(v.Data, 10, 64) + v2, _ := strconv.ParseInt(other.Data, 10, 64) + if v1 >= v2 { + return false + } + return true + case "float64": + v1, _ := strconv.ParseFloat(v.Data, 64) + v2, _ := strconv.ParseFloat(other.Data, 64) + if v1 >= v2 { + return false + } + return true + } + return false +} + type operander interface { Operands() []Expr } diff --git a/rule/expr_test.go b/rule/expr_test.go index a84db5e..6ac275e 100644 --- a/rule/expr_test.go +++ b/rule/expr_test.go @@ -408,6 +408,79 @@ func TestFNV(t *testing.T) { } } +func TestLT(t *testing.T) { + cases := []struct { + name string + m1 mockExpr + m2 mockExpr + expected *rule.Value + }{ + { + name: "String: true", + m1: mockExpr{val: rule.StringValue("abc")}, + m2: mockExpr{val: rule.StringValue("abd")}, + expected: rule.BoolValue(true), + }, + { + name: "String: false", + m1: mockExpr{val: rule.StringValue("abd")}, + m2: mockExpr{val: rule.StringValue("abc")}, + expected: rule.BoolValue(false), + }, + { + name: "Bool: true", + m1: mockExpr{val: rule.BoolValue(false)}, + m2: mockExpr{val: rule.BoolValue(true)}, + expected: rule.BoolValue(true), + }, + { + name: "Bool: false#1", + m1: mockExpr{val: rule.BoolValue(false)}, + m2: mockExpr{val: rule.BoolValue(false)}, + expected: rule.BoolValue(false), + }, + { + name: "Bool: false#2", + m1: mockExpr{val: rule.BoolValue(true)}, + m2: mockExpr{val: rule.BoolValue(false)}, + expected: rule.BoolValue(false), + }, + { + name: "Int64: true", + m1: mockExpr{val: rule.Int64Value(11)}, + m2: mockExpr{val: rule.Int64Value(12)}, + expected: rule.BoolValue(true), + }, + { + name: "Int64: false", + m1: mockExpr{val: rule.Int64Value(12)}, + m2: mockExpr{val: rule.Int64Value(12)}, + expected: rule.BoolValue(false), + }, + { + name: "Float64: true", + m1: mockExpr{val: rule.Float64Value(12.0)}, + m2: mockExpr{val: rule.Float64Value(12.1)}, + expected: rule.BoolValue(true), + }, + { + name: "Float64: false", + m1: mockExpr{val: rule.Float64Value(12.1)}, + m2: mockExpr{val: rule.Float64Value(12.0)}, + expected: rule.BoolValue(false), + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + lt := rule.LT(&tc.m1, &tc.m2) + val, err := lt.Eval(nil) + require.NoError(t, err) + require.Equal(t, tc.expected, val) + }) + } +} + func TestPercentile(t *testing.T) { // "Bob Dylan" is in the 96th percentile, so this is true v1 := rule.StringValue("Bob Dylan") diff --git a/rule/json.go b/rule/json.go index 334a097..a1425c4 100644 --- a/rule/json.go +++ b/rule/json.go @@ -121,6 +121,10 @@ func unmarshalExpr(kind string, data []byte) (Expr, error) { var fnv exprFNV e = &fnv err = fnv.UnmarshalJSON(data) + case "lt": + var lt exprLT + e = < + err = lt.UnmarshalJSON(data) default: err = errors.New("unknown expression kind " + kind) } From aab248006b6c9581f7484ceb0a41613e0f34e31d Mon Sep 17 00:00:00 2001 From: Yasss Date: Tue, 2 Apr 2019 16:54:13 +0200 Subject: [PATCH 3/7] Add LTE operator --- rule/expr.go | 73 +++++++++++++++++++++++++++++++++++ rule/expr_test.go | 97 +++++++++++++++++++++++++++++++++++++++++++++++ rule/json.go | 4 ++ 3 files changed, 174 insertions(+) diff --git a/rule/expr.go b/rule/expr.go index 09f0800..a2fcf3c 100644 --- a/rule/expr.go +++ b/rule/expr.go @@ -428,6 +428,46 @@ func (n *exprLT) Eval(params Params) (*Value, error) { return BoolValue(true), nil } +type exprLTE struct { + operator +} + +// LTE creates an expression that takes at least two operands and +// evaluates to true if each successive operand has a lower or equal value +// compared to the next. +func LTE(v1, v2 Expr, vN ...Expr) Expr { + return &exprLTE{ + operator: operator{ + kind: "lte", + operands: append([]Expr{v1, v2}, vN...), + }, + } +} + +func (n *exprLTE) Eval(params Params) (*Value, error) { + if len(n.operands) < 2 { + return nil, errors.New("invalid number of operands in LTE func") + } + + vA, err := n.operands[0].Eval(params) + if err != nil { + return nil, err + } + + for i := 1; i < len(n.operands); i++ { + vB, err := n.operands[i].Eval(params) + if err != nil { + return nil, err + } + + if !vA.LTE(vB) { + return BoolValue(false), nil + } + } + + return BoolValue(true), nil +} + // Param is an expression used to select a parameter passed during evaluation and return its corresponding value. type Param struct { Kind string `json:"kind"` @@ -684,6 +724,39 @@ func (v *Value) LT(other *Value) bool { return false } +// LTE reports whether v is less or equal than other. +func (v *Value) LTE(other *Value) bool { + switch v.Type { + case "bool": + v1, _ := strconv.ParseBool(v.Data) + v2, _ := strconv.ParseBool(other.Data) + if v1 && !v2 { + return false + } + return true + case "string": + if v.Data > other.Data { + return false + } + return true + case "int64": + v1, _ := strconv.ParseInt(v.Data, 10, 64) + v2, _ := strconv.ParseInt(other.Data, 10, 64) + if v1 > v2 { + return false + } + return true + case "float64": + v1, _ := strconv.ParseFloat(v.Data, 64) + v2, _ := strconv.ParseFloat(other.Data, 64) + if v1 > v2 { + return false + } + return true + } + return false +} + type operander interface { Operands() []Expr } diff --git a/rule/expr_test.go b/rule/expr_test.go index 6ac275e..a28c611 100644 --- a/rule/expr_test.go +++ b/rule/expr_test.go @@ -498,6 +498,103 @@ func TestPercentile(t *testing.T) { require.Equal(t, rule.BoolValue(false), res) } +func TestLTE(t *testing.T) { + cases := []struct { + name string + m1 mockExpr + m2 mockExpr + expected *rule.Value + }{ + { + name: "String: true#1", + m1: mockExpr{val: rule.StringValue("abc")}, + m2: mockExpr{val: rule.StringValue("abc")}, + expected: rule.BoolValue(true), + }, + { + name: "String: true#2", + m1: mockExpr{val: rule.StringValue("abc")}, + m2: mockExpr{val: rule.StringValue("abd")}, + expected: rule.BoolValue(true), + }, + { + name: "String: false", + m1: mockExpr{val: rule.StringValue("abd")}, + m2: mockExpr{val: rule.StringValue("abc")}, + expected: rule.BoolValue(false), + }, + { + name: "Bool: true#1", + m1: mockExpr{val: rule.BoolValue(false)}, + m2: mockExpr{val: rule.BoolValue(true)}, + expected: rule.BoolValue(true), + }, + { + name: "Bool: true#2", + m1: mockExpr{val: rule.BoolValue(false)}, + m2: mockExpr{val: rule.BoolValue(false)}, + expected: rule.BoolValue(true), + }, + { + name: "Bool: true#3", + m1: mockExpr{val: rule.BoolValue(true)}, + m2: mockExpr{val: rule.BoolValue(true)}, + expected: rule.BoolValue(true), + }, + { + name: "Bool: false", + m1: mockExpr{val: rule.BoolValue(true)}, + m2: mockExpr{val: rule.BoolValue(false)}, + expected: rule.BoolValue(false), + }, + { + name: "Int64: true#1", + m1: mockExpr{val: rule.Int64Value(11)}, + m2: mockExpr{val: rule.Int64Value(12)}, + expected: rule.BoolValue(true), + }, + { + name: "Int64: true#2", + m1: mockExpr{val: rule.Int64Value(12)}, + m2: mockExpr{val: rule.Int64Value(12)}, + expected: rule.BoolValue(true), + }, + { + name: "Int64: false", + m1: mockExpr{val: rule.Int64Value(12)}, + m2: mockExpr{val: rule.Int64Value(11)}, + expected: rule.BoolValue(false), + }, + { + name: "Float64: true#1", + m1: mockExpr{val: rule.Float64Value(12.0)}, + m2: mockExpr{val: rule.Float64Value(12.1)}, + expected: rule.BoolValue(true), + }, + { + name: "Float64: true#2", + m1: mockExpr{val: rule.Float64Value(12.1)}, + m2: mockExpr{val: rule.Float64Value(12.1)}, + expected: rule.BoolValue(true), + }, + { + name: "Float64: false", + m1: mockExpr{val: rule.Float64Value(12.1)}, + m2: mockExpr{val: rule.Float64Value(12.0)}, + expected: rule.BoolValue(false), + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + lte := rule.LTE(&tc.m1, &tc.m2) + val, err := lte.Eval(nil) + require.NoError(t, err) + require.Equal(t, tc.expected, val) + }) + } +} + func TestParam(t *testing.T) { t.Run("OK", func(t *testing.T) { v := rule.StringParam("foo") diff --git a/rule/json.go b/rule/json.go index a1425c4..4584fc0 100644 --- a/rule/json.go +++ b/rule/json.go @@ -125,6 +125,10 @@ func unmarshalExpr(kind string, data []byte) (Expr, error) { var lt exprLT e = < err = lt.UnmarshalJSON(data) + case "lte": + var lte exprLTE + e = <e + err = lte.UnmarshalJSON(data) default: err = errors.New("unknown expression kind " + kind) } From aa200a0d821b08a13ab58d000b88a074fc1233aa Mon Sep 17 00:00:00 2001 From: Yasss Date: Tue, 2 Apr 2019 17:05:25 +0200 Subject: [PATCH 4/7] Add tests for json encoding/decoding part --- rule/json_test.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/rule/json_test.go b/rule/json_test.go index d936ab0..6109854 100644 --- a/rule/json_test.go +++ b/rule/json_test.go @@ -54,7 +54,11 @@ func TestUnmarshalExpr(t *testing.T) { {"not", []byte(`{"kind":"not","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprNot)}, {"and", []byte(`{"kind":"and","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprAnd)}, {"or", []byte(`{"kind":"or","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprOr)}, - {"or", []byte(`{"kind":"percentile","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprOr)}, + {"percentile", []byte(`{"kind":"percentile","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprPercentile)}, + {"gt", []byte(`{"kind":"gt","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprGT)}, + {"gte", []byte(`{"kind":"gte","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprGTE)}, + {"lt", []byte(`{"kind":"lt","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprLT)}, + {"lte", []byte(`{"kind":"lte","operands": [{"kind": "value"}, {"kind": "param"}]}`), new(exprLTE)}, {"param", []byte(`{"kind":"param"}`), new(Param)}, {"value", []byte(`{"kind":"value"}`), new(Value)}, } @@ -142,6 +146,22 @@ func TestRuleUnmarshalling(t *testing.T) { StringValue("Bob Dylan"), ), ), + GT( + Int64Value(11), + Int64Value(10), + ), + GTE( + Int64Value(11), + Int64Value(11), + ), + LT( + Int64Value(10), + Int64Value(11), + ), + LTE( + Int64Value(10), + Int64Value(10), + ), ), True(), ), From a934982bd821bf36b69f910c1a58740b6b7746e7 Mon Sep 17 00:00:00 2001 From: Yasss Date: Wed, 3 Apr 2019 16:32:50 +0200 Subject: [PATCH 5/7] Reordering things ... (Monk habit) --- rule/expr.go | 138 +++++++++++++++++++++++++-------------------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/rule/expr.go b/rule/expr.go index a2fcf3c..20ce4eb 100644 --- a/rule/expr.go +++ b/rule/expr.go @@ -319,21 +319,6 @@ func (n *exprGTE) Eval(params Params) (*Value, error) { return BoolValue(true), nil } -type exprFNV struct { - operator -} - -// FNV returns an Integer hash of any value it is provided. It uses -// the Fowler-Noll-Vo non-cryptographic hash function. -func FNV(v Expr) Expr { - return &exprFNV{ - operator: operator{ - kind: "fnv", - operands: []Expr{v}, - }, - } -} - type exprLT struct { operator } @@ -350,60 +335,6 @@ func LT(v1, v2 Expr, vN ...Expr) Expr { } } -func (n *exprFNV) Eval(params Params) (*Value, error) { - if len(n.operands) != 1 { - return nil, errors.New("invalid number of operands in FNV func") - } - - h32 := fnv.New32() - op := n.operands[0] - v, err := op.Eval(params) - if err != nil { - return nil, err - } - _, err = h32.Write([]byte(v.Data)) - if err != nil { - return nil, err - } - return Int64Value(int64(h32.Sum32())), nil -} - -type exprPercentile struct { - operator -} - -// Percentile indicates whether the provided value is within a given -// percentile of the group of all such values. It is intended to be -// used to assign values to groups for experimentation. -func Percentile(v, p Expr) Expr { - return &exprPercentile{ - operator: operator{ - kind: "percentile", - operands: []Expr{v, p}, - }, - } -} - -func (n *exprPercentile) Eval(params Params) (*Value, error) { - if len(n.operands) != 2 { - return nil, errors.New("invalid number of operands in Percentile func") - } - - hash := FNV(n.operands[0]) - v, err := exprToInt64(hash, params) - if err != nil { - return nil, err - } - p, err := exprToInt64(n.operands[1], params) - if err != nil { - return nil, err - } - if (v % 100) <= p { - return BoolValue(true), nil - } - return BoolValue(false), nil -} - func (n *exprLT) Eval(params Params) (*Value, error) { if len(n.operands) < 2 { return nil, errors.New("invalid number of operands in LT func") @@ -468,6 +399,75 @@ func (n *exprLTE) Eval(params Params) (*Value, error) { return BoolValue(true), nil } +type exprFNV struct { + operator +} + +// FNV returns an Integer hash of any value it is provided. It uses +// the Fowler-Noll-Vo non-cryptographic hash function. +func FNV(v Expr) Expr { + return &exprFNV{ + operator: operator{ + kind: "fnv", + operands: []Expr{v}, + }, + } +} + +func (n *exprFNV) Eval(params Params) (*Value, error) { + if len(n.operands) != 1 { + return nil, errors.New("invalid number of operands in FNV func") + } + + h32 := fnv.New32() + op := n.operands[0] + v, err := op.Eval(params) + if err != nil { + return nil, err + } + _, err = h32.Write([]byte(v.Data)) + if err != nil { + return nil, err + } + return Int64Value(int64(h32.Sum32())), nil +} + +type exprPercentile struct { + operator +} + +// Percentile indicates whether the provided value is within a given +// percentile of the group of all such values. It is intended to be +// used to assign values to groups for experimentation. +func Percentile(v, p Expr) Expr { + return &exprPercentile{ + operator: operator{ + kind: "percentile", + operands: []Expr{v, p}, + }, + } +} + +func (n *exprPercentile) Eval(params Params) (*Value, error) { + if len(n.operands) != 2 { + return nil, errors.New("invalid number of operands in Percentile func") + } + + hash := FNV(n.operands[0]) + v, err := exprToInt64(hash, params) + if err != nil { + return nil, err + } + p, err := exprToInt64(n.operands[1], params) + if err != nil { + return nil, err + } + if (v % 100) <= p { + return BoolValue(true), nil + } + return BoolValue(false), nil +} + // Param is an expression used to select a parameter passed during evaluation and return its corresponding value. type Param struct { Kind string `json:"kind"` From 5b13488044b0c7020613489a7de78b29cde06ff2 Mon Sep 17 00:00:00 2001 From: Yasss Date: Wed, 3 Apr 2019 17:55:02 +0200 Subject: [PATCH 6/7] Check errors when convert a data Value to a concrete type --- rule/expr.go | 229 +++++++++++++++++++++++++------------ rule/expr_internal_test.go | 95 +++++++++++++++ 2 files changed, 252 insertions(+), 72 deletions(-) create mode 100644 rule/expr_internal_test.go diff --git a/rule/expr.go b/rule/expr.go index 20ce4eb..296b96c 100644 --- a/rule/expr.go +++ b/rule/expr.go @@ -2,6 +2,7 @@ package rule import ( "errors" + "fmt" "go/token" "strconv" @@ -271,7 +272,12 @@ func (n *exprGT) Eval(params Params) (*Value, error) { return nil, err } - if !vA.GT(vB) { + res, err := vA.GT(vB) + if err != nil { + return nil, err + } + + if !res { return BoolValue(false), nil } } @@ -311,7 +317,12 @@ func (n *exprGTE) Eval(params Params) (*Value, error) { return nil, err } - if !vA.GTE(vB) { + res, err := vA.GTE(vB) + if err != nil { + return nil, err + } + + if !res { return BoolValue(false), nil } } @@ -351,7 +362,12 @@ func (n *exprLT) Eval(params Params) (*Value, error) { return nil, err } - if !vA.LT(vB) { + res, err := vA.LT(vB) + if err != nil { + return nil, err + } + + if !res { return BoolValue(false), nil } } @@ -391,7 +407,12 @@ func (n *exprLTE) Eval(params Params) (*Value, error) { return nil, err } - if !vA.LTE(vB) { + res, err := vA.LTE(vB) + if err != nil { + return nil, err + } + + if !res { return BoolValue(false), nil } } @@ -614,147 +635,211 @@ func (v *Value) Equal(other *Value) bool { } // GT reports whether v is greater than other. -func (v *Value) GT(other *Value) bool { +func (v *Value) GT(other *Value) (bool, error) { switch v.Type { case "bool": - v1, _ := strconv.ParseBool(v.Data) + v1, v2, err := parseBoolValues(v, other) + if err != nil { + return false, err + } + if !v1 { // If v1 is False then it's not greater than v2, and we can be done already. - return false + return false, nil } - - v2, _ := strconv.ParseBool(other.Data) if v2 { // If v2 is True then v1 can't be greater than it.. - return false + return false, nil } - return true + return true, nil case "string": if v.Data <= other.Data { - return false + return false, nil } - return true + return true, nil case "int64": - v1, _ := strconv.ParseInt(v.Data, 10, 64) - v2, _ := strconv.ParseInt(other.Data, 10, 64) + v1, v2, err := parseInt64Values(v, other) + if err != nil { + return false, err + } + if v1 <= v2 { - return false + return false, nil } - return true + return true, nil case "float64": - v1, _ := strconv.ParseFloat(v.Data, 64) - v2, _ := strconv.ParseFloat(other.Data, 64) + v1, v2, err := parseFloat64Values(v, other) + if err != nil { + return false, err + } + if v1 <= v2 { - return false + return false, nil } - return true + return true, nil } - return false + return false, fmt.Errorf("unknown Value type: %s", v.Type) } // GTE reports whether v is greater or equal than other. -func (v *Value) GTE(other *Value) bool { +func (v *Value) GTE(other *Value) (bool, error) { switch v.Type { case "bool": - v1, _ := strconv.ParseBool(v.Data) - v2, _ := strconv.ParseBool(other.Data) + v1, v2, err := parseBoolValues(v, other) + if err != nil { + return false, err + } + if !v1 && v2 { - return false + return false, nil } - return true + return true, nil case "string": if v.Data < other.Data { - return false + return false, nil } - return true + return true, nil case "int64": - v1, _ := strconv.ParseInt(v.Data, 10, 64) - v2, _ := strconv.ParseInt(other.Data, 10, 64) + v1, v2, err := parseInt64Values(v, other) + if err != nil { + return false, err + } + if v1 < v2 { - return false + return false, nil } - return true + return true, nil case "float64": - v1, _ := strconv.ParseFloat(v.Data, 64) - v2, _ := strconv.ParseFloat(other.Data, 64) + v1, v2, err := parseFloat64Values(v, other) + if err != nil { + return false, err + } + if v1 < v2 { - return false + return false, nil } - return true + return true, nil } - return false + return false, fmt.Errorf("unknown Value type: %s", v.Type) } // LT reports whether v is less than other. -func (v *Value) LT(other *Value) bool { +func (v *Value) LT(other *Value) (bool, error) { switch v.Type { case "bool": - v1, _ := strconv.ParseBool(v.Data) + v1, v2, err := parseBoolValues(v, other) + if err != nil { + return false, err + } + if v1 { // If v1 is True then it's not less than v2, and we can be done already. - return false + return false, nil } - - v2, _ := strconv.ParseBool(other.Data) if !v2 { // If v2 is False then v1 can't be less than it.. - return false + return false, nil } - return true + return true, nil case "string": if v.Data >= other.Data { - return false + return false, nil } - return true + return true, nil case "int64": - v1, _ := strconv.ParseInt(v.Data, 10, 64) - v2, _ := strconv.ParseInt(other.Data, 10, 64) + v1, v2, err := parseInt64Values(v, other) + if err != nil { + return false, err + } + if v1 >= v2 { - return false + return false, nil } - return true + return true, nil case "float64": - v1, _ := strconv.ParseFloat(v.Data, 64) - v2, _ := strconv.ParseFloat(other.Data, 64) + v1, v2, err := parseFloat64Values(v, other) + if err != nil { + return false, err + } + if v1 >= v2 { - return false + return false, nil } - return true + return true, nil } - return false + return false, fmt.Errorf("unknown Value type: %s", v.Type) } // LTE reports whether v is less or equal than other. -func (v *Value) LTE(other *Value) bool { +func (v *Value) LTE(other *Value) (bool, error) { switch v.Type { case "bool": - v1, _ := strconv.ParseBool(v.Data) - v2, _ := strconv.ParseBool(other.Data) + v1, v2, err := parseBoolValues(v, other) + if err != nil { + return false, err + } + if v1 && !v2 { - return false + return false, nil } - return true + return true, nil case "string": if v.Data > other.Data { - return false + return false, nil } - return true + return true, nil case "int64": - v1, _ := strconv.ParseInt(v.Data, 10, 64) - v2, _ := strconv.ParseInt(other.Data, 10, 64) + v1, v2, err := parseInt64Values(v, other) + if err != nil { + return false, nil + } + if v1 > v2 { - return false + return false, nil } - return true + return true, nil case "float64": - v1, _ := strconv.ParseFloat(v.Data, 64) - v2, _ := strconv.ParseFloat(other.Data, 64) + v1, v2, err := parseFloat64Values(v, other) + if err != nil { + return false, err + } + if v1 > v2 { - return false + return false, nil } - return true + return true, nil + } + return false, fmt.Errorf("unknown Value type: %s", v.Type) +} + +func parseBoolValues(v1, v2 *Value) (b1, b2 bool, err error) { + if b1, err = strconv.ParseBool(v1.Data); err != nil { + return + } + if b2, err = strconv.ParseBool(v2.Data); err != nil { + return + } + return +} + +func parseInt64Values(v1, v2 *Value) (i1, i2 int64, err error) { + if i1, err = strconv.ParseInt(v1.Data, 10, 64); err != nil { + return + } + if i2, err = strconv.ParseInt(v2.Data, 10, 64); err != nil { + return + } + return +} + +func parseFloat64Values(v1, v2 *Value) (f1, f2 float64, err error) { + if f1, err = strconv.ParseFloat(v1.Data, 64); err != nil { + return + } + if f2, err = strconv.ParseFloat(v2.Data, 64); err != nil { + return } - return false + return } type operander interface { diff --git a/rule/expr_internal_test.go b/rule/expr_internal_test.go new file mode 100644 index 0000000..ee49500 --- /dev/null +++ b/rule/expr_internal_test.go @@ -0,0 +1,95 @@ +package rule + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseBoolValues(t *testing.T) { + t.Run("OK", func(t *testing.T) { + v1 := newValue("bool", "true") + v2 := newValue("bool", "false") + + b1, b2, err := parseBoolValues(v1, v2) + require.NoError(t, err) + require.True(t, b1) + require.False(t, b2) + }) + + t.Run("Fail 1st Value", func(t *testing.T) { + v1 := newValue("bool", "foo") + v2 := newValue("bool", "false") + _, _, err := parseBoolValues(v1, v2) + require.Error(t, err) + require.Equal(t, err.Error(), `strconv.ParseBool: parsing "foo": invalid syntax`) + }) + + t.Run("Fail 2nd Value", func(t *testing.T) { + v1 := newValue("bool", "true") + v2 := newValue("bool", "bar") + _, _, err := parseBoolValues(v1, v2) + require.Error(t, err) + require.Equal(t, err.Error(), `strconv.ParseBool: parsing "bar": invalid syntax`) + }) +} + +func TestParseInt64Values(t *testing.T) { + t.Run("OK", func(t *testing.T) { + v1 := newValue("int64", "123") + v2 := newValue("int64", "456") + + i1, i2, err := parseInt64Values(v1, v2) + require.NoError(t, err) + require.Equal(t, int64(123), i1) + require.Equal(t, int64(456), i2) + }) + + t.Run("Fail 1st Value", func(t *testing.T) { + v1 := newValue("int64", "foo") + v2 := newValue("int64", "456") + + _, _, err := parseInt64Values(v1, v2) + require.Error(t, err) + require.Equal(t, err.Error(), `strconv.ParseInt: parsing "foo": invalid syntax`) + }) + + t.Run("Fail 2nd Value", func(t *testing.T) { + v1 := newValue("int64", "123") + v2 := newValue("int64", "bar") + + _, _, err := parseInt64Values(v1, v2) + require.Error(t, err) + require.Equal(t, err.Error(), `strconv.ParseInt: parsing "bar": invalid syntax`) + }) +} + +func TestParseFloat64Values(t *testing.T) { + t.Run("OK", func(t *testing.T) { + v1 := newValue("float64", "12.3") + v2 := newValue("float64", "45.6") + + f1, f2, err := parseFloat64Values(v1, v2) + require.NoError(t, err) + require.Equal(t, float64(12.3), f1) + require.Equal(t, float64(45.6), f2) + }) + + t.Run("Fail 1st Value", func(t *testing.T) { + v1 := newValue("float64", "foo") + v2 := newValue("float64", "45.6") + + _, _, err := parseFloat64Values(v1, v2) + require.Error(t, err) + require.Equal(t, err.Error(), `strconv.ParseFloat: parsing "foo": invalid syntax`) + }) + + t.Run("Fail 2nd Value", func(t *testing.T) { + v1 := newValue("float64", "12.3") + v2 := newValue("float64", "bar") + + _, _, err := parseFloat64Values(v1, v2) + require.Error(t, err) + require.Equal(t, err.Error(), `strconv.ParseFloat: parsing "bar": invalid syntax`) + }) +} From 5999a2f30b1b796d807b5e9b6ad3ffdc84a48a66 Mon Sep 17 00:00:00 2001 From: Yasss Date: Wed, 3 Apr 2019 18:02:47 +0200 Subject: [PATCH 7/7] Remove useless if block --- rule/expr.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rule/expr.go b/rule/expr.go index 296b96c..34524e9 100644 --- a/rule/expr.go +++ b/rule/expr.go @@ -816,9 +816,7 @@ func parseBoolValues(v1, v2 *Value) (b1, b2 bool, err error) { if b1, err = strconv.ParseBool(v1.Data); err != nil { return } - if b2, err = strconv.ParseBool(v2.Data); err != nil { - return - } + b2, err = strconv.ParseBool(v2.Data) return } @@ -826,9 +824,7 @@ func parseInt64Values(v1, v2 *Value) (i1, i2 int64, err error) { if i1, err = strconv.ParseInt(v1.Data, 10, 64); err != nil { return } - if i2, err = strconv.ParseInt(v2.Data, 10, 64); err != nil { - return - } + i2, err = strconv.ParseInt(v2.Data, 10, 64) return } @@ -836,9 +832,7 @@ func parseFloat64Values(v1, v2 *Value) (f1, f2 float64, err error) { if f1, err = strconv.ParseFloat(v1.Data, 64); err != nil { return } - if f2, err = strconv.ParseFloat(v2.Data, 64); err != nil { - return - } + f2, err = strconv.ParseFloat(v2.Data, 64) return }