From 32bf318d9b79eaa1f77ab32dd6d998cefb35b287 Mon Sep 17 00:00:00 2001 From: Eric Chlebek Date: Wed, 22 Nov 2017 17:07:40 -0800 Subject: [PATCH] Upgrade govaluate, avoid generating structs. Now that govaluate has been modified to recursively query Parameters, we can return a lazy tree of AnyParameters instead of dynamically generating structs. This saves quite a bit of code, CPU time, and memory allocations. Old struct generation BenchmarkQueryGovaluateSimple-4 2069 ns/op 992 B/op 25 allocs/op BenchmarkQueryGovaluateComplex-4 6212 ns/op 2544 B/op 75 allocs/op New lazy json parsing BenchmarkQueryGovaluateSimple-4 1404 ns/op 320 B/op 18 allocs/op BenchmarkQueryGovaluateComplex-4 2617 ns/op 512 B/op 30 allocs/op Refs #586 --- types/dynamic/dynamic.go | 94 +++++-------------- types/dynamic/dynamic_test.go | 58 +++++------- .../Knetic/govaluate/evaluationStage.go | 12 +++ vendor/github.com/Knetic/govaluate/parsing.go | 10 -- 4 files changed, 57 insertions(+), 117 deletions(-) diff --git a/types/dynamic/dynamic.go b/types/dynamic/dynamic.go index 0084dc2754..f7172166b6 100644 --- a/types/dynamic/dynamic.go +++ b/types/dynamic/dynamic.go @@ -28,41 +28,33 @@ type Attributer interface { // field. GetField is case-sensitive, but extended attribute names will be // converted to CamelCaps. func GetField(v Attributer, name string) (interface{}, error) { - strukt := reflect.Indirect(reflect.ValueOf(v)) - if kind := strukt.Kind(); kind != reflect.Struct { - return nil, fmt.Errorf("invalid type (want struct): %v", kind) - } - fields := getFields(strukt) - field, ok := fields[name] - if ok { - return field.Value.Interface(), nil + if s := string([]rune(name)[0]); strings.Title(s) == s { + // Exported fields are always upper-cased for the first rune + strukt := reflect.Indirect(reflect.ValueOf(v)) + if kind := strukt.Kind(); kind != reflect.Struct { + return nil, fmt.Errorf("invalid type (want struct): %v", kind) + } + fields := getFields(strukt) + field, ok := fields[name] + if ok { + return field.Value.Interface(), nil + } } // If we get here, we are dealing with extended attributes. - return getExtendedAttribute(v.Attributes().data, name) + any := AnyParameters{any: jsoniter.Get(v.Attributes().data)} + return any.Get(name) +} + +// Connect jsoniter.Any to govaluate.Parameters +type AnyParameters struct { + any jsoniter.Any } -// getExtendedAttribute dynamically builds a concrete type. If the concrete -// type is a composite type, then it will either be a struct or a slice. -func getExtendedAttribute(msg []byte, name string) (interface{}, error) { - any := jsoniter.Get(msg, name) +func (p AnyParameters) Get(name string) (interface{}, error) { + any := p.any.Get(name) if err := any.LastError(); err != nil { - lowerName := fmt.Sprintf("%s%s", strings.ToLower(string(name[0])), name[1:]) - if name != lowerName { - // fall back to lower-case name - return getExtendedAttribute(msg, lowerName) - } return nil, err } - if any.GetInterface() == nil { - // Fall back to lower-case name - lowerName := fmt.Sprintf("%s%s", strings.ToLower(string(name[0])), name[1:]) - any = jsoniter.Get(msg, lowerName) - } - value, err := anyToValue(any) - return value, err -} - -func anyToValue(any jsoniter.Any) (interface{}, error) { switch any.ValueType() { case jsoniter.InvalidValue: return nil, fmt.Errorf("dynamic: %s", any.LastError()) @@ -75,53 +67,13 @@ func anyToValue(any jsoniter.Any) (interface{}, error) { case jsoniter.BoolValue: return any.ToBool(), nil case jsoniter.ArrayValue: - return buildSliceAny(any) + return any.GetInterface(), any.LastError() case jsoniter.ObjectValue: - return buildStructAny(any) + return AnyParameters{any: any}, any.LastError() default: - return nil, fmt.Errorf("dynamic: unrecognized value type! %d", any.ValueType()) - } -} - -// buildSliceAny dynamically builds a slice from a jsoniter.Any -func buildSliceAny(any jsoniter.Any) (interface{}, error) { - n := any.Size() - result := make([]interface{}, 0, n) - for i := 0; i < n; i++ { - value, err := anyToValue(any.Get(i)) - if err != nil { - return nil, err - } - result = append(result, value) - } - return result, nil -} - -// buildStructAny dynamically builds a struct from a jsoniter.Any -func buildStructAny(any jsoniter.Any) (interface{}, error) { - keys := any.Keys() - fields := make([]reflect.StructField, 0, len(keys)) - values := make([]interface{}, 0, len(keys)) - for _, key := range keys { - value, err := anyToValue(any.Get(key)) - if err != nil { - return nil, err - } - values = append(values, value) - fields = append(fields, reflect.StructField{ - Name: strings.Title(key), - Type: reflect.TypeOf(value), - }) - } - structType := reflect.StructOf(fields) - structPtr := reflect.New(structType) - structVal := reflect.Indirect(structPtr) - for i, value := range values { - field := structVal.Field(i) - field.Set(reflect.ValueOf(value)) + return nil, fmt.Errorf("dynamic: unrecognized value type! %d", p.any.ValueType()) } - return structVal.Interface(), nil } // getFields gets a map of struct fields by name from a reflect.Value diff --git a/types/dynamic/dynamic_test.go b/types/dynamic/dynamic_test.go index f90a491285..b7a53532f3 100644 --- a/types/dynamic/dynamic_test.go +++ b/types/dynamic/dynamic_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/Knetic/govaluate" + jsoniter "github.com/json-iterator/go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -115,7 +116,7 @@ func TestExtractEmptyExtendedAttributes(t *testing.T) { var m MyType attrs, err := extractExtendedAttributes(m, msg) - require.Nil(err) + require.NoError(err) assert.Equal([]byte("{}"), attrs.data) } @@ -127,7 +128,7 @@ func TestExtractExtendedAttributes(t *testing.T) { var m MyType attrs, err := extractExtendedAttributes(m, msg) - require.Nil(err) + require.NoError(err) assert.Equal([]byte(`{"extendedattr":"such extended"}`), attrs.data) } @@ -144,7 +145,7 @@ func TestMarshal(t *testing.T) { } b, err := Marshal(m) - require.Nil(err) + require.NoError(err) assert.Equal(expBytes, b) } @@ -155,6 +156,9 @@ func TestGetField(t *testing.T) { attrs: Attributes{data: []byte(`{"a":"a","b":1,"c":2.0,"d":true,"e":null,"foo":{"hello":5},"bar":[true,10.5]}`)}, } + fooAny := jsoniter.Get([]byte(`{"hello":5}`)) + require.NoError(t, fooAny.LastError()) + tests := []struct { Field string Exp interface{} @@ -187,10 +191,8 @@ func TestGetField(t *testing.T) { }, { Field: "foo", - Exp: struct { - Hello float64 - }{Hello: 5.0}, - Kind: reflect.Struct, + Exp: AnyParameters{any: fooAny}, + Kind: reflect.Struct, }, { Field: "bar", @@ -202,7 +204,7 @@ func TestGetField(t *testing.T) { for _, test := range tests { t.Run(test.Field, func(t *testing.T) { field, err := GetField(m, test.Field) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, test.Exp, field) assert.Equal(t, test.Kind, reflect.ValueOf(field).Kind()) }) @@ -215,19 +217,11 @@ func TestQueryGovaluateSimple(t *testing.T) { } expr, err := govaluate.NewEvaluableExpression("hello == 5") - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, expr) result, err := expr.Eval(m) - require.Nil(t, err) - require.Equal(t, true, result) - - expr, err = govaluate.NewEvaluableExpression("Hello == 5") - require.Nil(t, err) - require.NotNil(t, expr) - - result, err = expr.Eval(m) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, true, result) } @@ -237,7 +231,7 @@ func BenchmarkQueryGovaluateSimple(b *testing.B) { } expr, err := govaluate.NewEvaluableExpression("hello == 5") - require.Nil(b, err) + require.NoError(b, err) require.NotNil(b, expr) b.ResetTimer() @@ -251,28 +245,20 @@ func TestQueryGovaluateComplex(t *testing.T) { attrs: Attributes{data: []byte(`{"hello":{"foo":5,"bar":6.0}}`)}, } - expr, err := govaluate.NewEvaluableExpression("hello.Foo == 5") - require.Nil(t, err) + expr, err := govaluate.NewEvaluableExpression("hello.foo == 5") + require.NoError(t, err) require.NotNil(t, expr) result, err := expr.Eval(m) - require.Nil(t, err) - require.Equal(t, true, result) - - expr, err = govaluate.NewEvaluableExpression("Hello.Foo == 5") - require.Nil(t, err) - require.NotNil(t, expr) - - result, err = expr.Eval(m) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, true, result) - expr, err = govaluate.NewEvaluableExpression("Hello.Foo < Hello.Bar") - require.Nil(t, err) + expr, err = govaluate.NewEvaluableExpression("hello.foo < hello.bar") + require.NoError(t, err) require.NotNil(t, expr) result, err = expr.Eval(m) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, true, result) } @@ -282,7 +268,7 @@ func BenchmarkQueryGovaluateComplex(b *testing.B) { } expr, err := govaluate.NewEvaluableExpression("hello.Foo == 5") - require.Nil(b, err) + require.NoError(b, err) require.NotNil(b, expr) b.ResetTimer() @@ -295,10 +281,10 @@ func TestMarshalUnmarshal(t *testing.T) { data := []byte(`{"bar":null,"foo":"hello","a":10,"b":"c"}`) var m MyType err := json.Unmarshal(data, &m) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, MyType{Foo: "hello", attrs: Attributes{data: []byte(`{"a":10,"b":"c"}`)}}, m) b, err := json.Marshal(&m) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, data, b) } diff --git a/vendor/github.com/Knetic/govaluate/evaluationStage.go b/vendor/github.com/Knetic/govaluate/evaluationStage.go index 11ea587240..4f23809e1f 100644 --- a/vendor/github.com/Knetic/govaluate/evaluationStage.go +++ b/vendor/github.com/Knetic/govaluate/evaluationStage.go @@ -314,6 +314,13 @@ func makeAccessorStage(pair []string) evaluationOperator { }() for i := 1; i < len(pair); i++ { + if p, ok := value.(Parameters); ok { + value, err = p.Get(pair[i]) + if err != nil { + return nil, err + } + continue + } coreValue := reflect.ValueOf(value) @@ -325,6 +332,11 @@ func makeAccessorStage(pair []string) evaluationOperator { coreValue = coreValue.Elem() } + if field, ok := coreValue.Type().FieldByName(pair[i]); ok && len(field.PkgPath) > 0 { + // According to reflect docs, if len(PkgPath) > 0 then the field is unexported + return nil, fmt.Errorf("Unable to access unexported field '%s'", field.PkgPath) + } + if coreValue.Kind() != reflect.Struct { return nil, errors.New("Unable to access '" + pair[i] + "', '" + pair[i-1] + "' is not a struct") } diff --git a/vendor/github.com/Knetic/govaluate/parsing.go b/vendor/github.com/Knetic/govaluate/parsing.go index 40c7ed2c47..f4e444d05e 100644 --- a/vendor/github.com/Knetic/govaluate/parsing.go +++ b/vendor/github.com/Knetic/govaluate/parsing.go @@ -189,16 +189,6 @@ func readToken(stream *lexerStream, state lexerState, functions map[string]Expre splits := strings.Split(tokenString, ".") tokenValue = splits - // check that none of them are unexported - for i := 1; i < len(splits); i++ { - - firstCharacter := getFirstRune(splits[i]) - - if unicode.ToUpper(firstCharacter) != firstCharacter { - errorMsg := fmt.Sprintf("Unable to access unexported field '%s' in token '%s'", splits[i], tokenString) - return ExpressionToken{}, errors.New(errorMsg), false - } - } } break }