Skip to content

Commit

Permalink
Improve package ergonomics and fix a bug.
Browse files Browse the repository at this point in the history 
* Break up Attributer interface into AttrGetter and AttrSetter.
* Get rid of the Attributes type in favour of simple []byte.
* Make AttrGetter method set match what protobuf might generate.
* When returning struct fields from GetField, use reflect.Indirect
  so that pointers are never returned.
* Make sure that extended attributes are never returned by GetField
  and never marshaled by Marshal.

Refs #586
  • Loading branch information
@echlebek
echlebek committed Nov 23, 2017
1 parent 5a16dbe commit 6a3137d
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 39 deletions.
64 changes: 40 additions & 24 deletions types/dynamic/dynamic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,26 @@ import (
jsoniter "github.com/json-iterator/go"
)

// Attributes hold arbitrary JSON-encoded data.
type Attributes struct {
data []byte
// AttrGetter is required to be implemented by types that need to work with
// GetField and Marshal.
type AttrGetter interface {
GetExtendedAttributes() []byte
}

// Attributer can be implemented to enable a type to work with the Marshal
// and Unmarshal functions in this package.
type Attributer interface {
Attributes() Attributes
SetAttributes(Attributes)
// AttrSetter is required to be implemented by types that need to work with
// Unmarshal.
type AttrSetter interface {
SetExtendedAttributes([]byte)
}

// GetField gets a field from v according to its name.
// If GetField doesn't find a struct field with the corresponding name, then
// it will try to dynamically find the corresponding item in the 'Extended'
// field. GetField is case-sensitive, but extended attribute names will be
// converted to CamelCaps.
func GetField(v Attributer, name string) (interface{}, error) {
func GetField(v AttrGetter, name string) (interface{}, error) {
extendedAttributes := v.GetExtendedAttributes()
extAttrPtr := &extendedAttributes[0]
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))
Expand All @@ -36,11 +38,19 @@ func GetField(v Attributer, name string) (interface{}, error) {
}
field := strukt.FieldByName(name)
if field.IsValid() {
return field.Interface(), nil
rval := reflect.Indirect(field).Interface()
if b, ok := rval.([]byte); ok {
// Make sure this field isn't the extended attributes
if extAttrPtr == &b[0] {
goto EXTENDED
}
}
return rval, nil
}
}
EXTENDED:
// If we get here, we are dealing with extended attributes.
any := AnyParameters{any: jsoniter.Get(v.Attributes().data)}
any := AnyParameters{any: jsoniter.Get(extendedAttributes)}
return any.Get(name)
}

Expand Down Expand Up @@ -112,7 +122,7 @@ func (s structField) jsonFieldName() (string, bool) {
return fieldName, omitEmpty
}

func getJSONFields(v reflect.Value) map[string]structField {
func getJSONFields(v reflect.Value, addressOfAttrs *byte) map[string]structField {
typ := v.Type()
numField := v.NumField()
result := make(map[string]structField, numField)
Expand All @@ -131,6 +141,11 @@ func getJSONFields(v reflect.Value) map[string]structField {
if sf.OmitEmpty && sf.IsEmpty() {
continue
}
if b, ok := reflect.Indirect(value).Interface().([]byte); ok {
if len(b) > 0 && &b[0] == addressOfAttrs {
continue
}
}
// sf is a valid JSON field.
result[sf.JSONName] = sf
}
Expand All @@ -140,16 +155,16 @@ func getJSONFields(v reflect.Value) map[string]structField {
// extractExtendedAttributes selects only extended attributes from msg. It will
// ignore any fields in msg that correspond to fields in v. v must be of kind
// reflect.Struct.
func extractExtendedAttributes(v interface{}, msg []byte) (Attributes, error) {
func extractExtendedAttributes(v interface{}, msg []byte) ([]byte, error) {
strukt := reflect.Indirect(reflect.ValueOf(v))
if kind := strukt.Kind(); kind != reflect.Struct {
return Attributes{}, fmt.Errorf("invalid type (want struct): %v", kind)
return nil, fmt.Errorf("invalid type (want struct): %v", kind)
}
fields := getJSONFields(strukt)
fields := getJSONFields(strukt, nil)
stream := jsoniter.NewStream(jsoniter.ConfigDefault, nil, 4096)
var anys map[string]jsoniter.Any
if err := jsoniter.Unmarshal(msg, &anys); err != nil {
return Attributes{}, err
return nil, err
}
stream.WriteObjectStart()
j := 0
Expand All @@ -167,12 +182,12 @@ func extractExtendedAttributes(v interface{}, msg []byte) (Attributes, error) {
any.WriteTo(stream)
}
stream.WriteObjectEnd()
return Attributes{data: stream.Buffer()}, nil
return stream.Buffer(), nil
}

// Unmarshal decodes msg into v, storing what fields it can into the basic
// fields of the struct, and storing the rest into Attributes.
func Unmarshal(msg []byte, v Attributer) error {
// fields of the struct, and storing the rest into its extended attributes.
func Unmarshal(msg []byte, v AttrSetter) error {
if _, ok := v.(json.Unmarshaler); ok {
// Can't safely call UnmarshalJSON here without potentially causing an
// infinite recursion. Copy the struct into a new type that doesn't
Expand Down Expand Up @@ -206,23 +221,23 @@ func Unmarshal(msg []byte, v Attributer) error {
if err != nil {
return err
}
v.SetAttributes(attrs)
v.SetExtendedAttributes(attrs)
return nil
}

// Marshal encodes the struct fields in v that are valid to encode.
// It also encodes any extended attributes that are defined. Marshal
// respects the encoding/json rules regarding exported fields, and tag
// semantics. If v's kind is not reflect.Struct, an error will be returned.
func Marshal(v Attributer) ([]byte, error) {
func Marshal(v AttrGetter) ([]byte, error) {
s := jsoniter.NewStream(jsoniter.ConfigDefault, nil, 4096)
s.WriteObjectStart()

if err := encodeStructFields(v, s); err != nil {
return nil, err
}

extended := v.Attributes().data
extended := v.GetExtendedAttributes()
if len(extended) > 0 {
if err := encodeExtendedFields(extended, s); err != nil {
return nil, err
Expand All @@ -234,12 +249,13 @@ func Marshal(v Attributer) ([]byte, error) {
return s.Buffer(), nil
}

func encodeStructFields(v interface{}, s *jsoniter.Stream) error {
func encodeStructFields(v AttrGetter, s *jsoniter.Stream) error {
strukt := reflect.Indirect(reflect.ValueOf(v))
if kind := strukt.Kind(); kind != reflect.Struct {
return fmt.Errorf("invalid type (want struct): %v", kind)
}
m := getJSONFields(strukt)
addressOfAttrs := &v.GetExtendedAttributes()[0]
m := getJSONFields(strukt, addressOfAttrs)
fields := make([]structField, 0, len(m))
for _, s := range m {
fields = append(fields, s)
Expand Down
41 changes: 26 additions & 15 deletions types/dynamic/dynamic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,17 @@ func TestGetJSONStructField(t *testing.T) {
ValidEmpty int `json:"validEmpty"`
InvalidEmpty int `json:"invalidEmpty,omitempty"`
Invalid int `json:"-"`
Attributes []byte
}{
Valid: 5,
invalidUnexported: 1,
ValidEmpty: 0,
InvalidEmpty: 0,
Invalid: 10,
Attributes: []byte(`"hello!"`),
}

fields := getJSONFields(reflect.ValueOf(test))
fields := getJSONFields(reflect.ValueOf(test), &test.Attributes[0])
require.Equal(2, len(fields))

field, ok := fields["valid"]
Expand All @@ -85,15 +87,15 @@ type MyType struct {
Foo string `json:"foo"`
Bar []MyType `json:"bar"`

attrs Attributes
Attrs []byte // note that this will not be marshalled directly, despite missing the `json"-"`!
}

func (m *MyType) Attributes() Attributes {
return m.attrs
func (m *MyType) GetExtendedAttributes() []byte {
return m.Attrs
}

func (m *MyType) SetAttributes(a Attributes) {
m.attrs = a
func (m *MyType) SetExtendedAttributes(a []byte) {
m.Attrs = a
}

func (m *MyType) Get(name string) (interface{}, error) {
Expand All @@ -117,7 +119,7 @@ func TestExtractEmptyExtendedAttributes(t *testing.T) {

attrs, err := extractExtendedAttributes(m, msg)
require.NoError(err)
assert.Equal([]byte("{}"), attrs.data)
assert.Equal([]byte("{}"), attrs)
}

func TestExtractExtendedAttributes(t *testing.T) {
Expand All @@ -129,7 +131,7 @@ func TestExtractExtendedAttributes(t *testing.T) {

attrs, err := extractExtendedAttributes(m, msg)
require.NoError(err)
assert.Equal([]byte(`{"extendedattr":"such extended"}`), attrs.data)
assert.Equal([]byte(`{"extendedattr":"such extended"}`), attrs)
}

func TestMarshal(t *testing.T) {
Expand All @@ -141,7 +143,7 @@ func TestMarshal(t *testing.T) {
m := &MyType{
Foo: "hello world!",
Bar: nil,
attrs: Attributes{data: extendedBytes},
Attrs: extendedBytes,
}

b, err := Marshal(m)
Expand All @@ -153,7 +155,7 @@ func TestGetField(t *testing.T) {
m := &MyType{
Foo: "hello",
Bar: []MyType{{Foo: "there"}},
attrs: Attributes{data: []byte(`{"a":"a","b":1,"c":2.0,"d":true,"e":null,"foo":{"hello":5},"bar":[true,10.5]}`)},
Attrs: []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}`))
Expand Down Expand Up @@ -213,7 +215,7 @@ func TestGetField(t *testing.T) {

func TestQueryGovaluateSimple(t *testing.T) {
m := &MyType{
attrs: Attributes{data: []byte(`{"hello":5}`)},
Attrs: []byte(`{"hello":5}`),
}

expr, err := govaluate.NewEvaluableExpression("hello == 5")
Expand All @@ -227,7 +229,7 @@ func TestQueryGovaluateSimple(t *testing.T) {

func BenchmarkQueryGovaluateSimple(b *testing.B) {
m := &MyType{
attrs: Attributes{data: []byte(`{"hello":5}`)},
Attrs: []byte(`{"hello":5}`),
}

expr, err := govaluate.NewEvaluableExpression("hello == 5")
Expand All @@ -242,7 +244,7 @@ func BenchmarkQueryGovaluateSimple(b *testing.B) {

func TestQueryGovaluateComplex(t *testing.T) {
m := &MyType{
attrs: Attributes{data: []byte(`{"hello":{"foo":5,"bar":6.0}}`)},
Attrs: []byte(`{"hello":{"foo":5,"bar":6.0}}`),
}

expr, err := govaluate.NewEvaluableExpression("hello.foo == 5")
Expand All @@ -264,7 +266,7 @@ func TestQueryGovaluateComplex(t *testing.T) {

func BenchmarkQueryGovaluateComplex(b *testing.B) {
m := &MyType{
attrs: Attributes{data: []byte(`{"hello":{"foo":5,"bar":6.0}}`)},
Attrs: []byte(`{"hello":{"foo":5,"bar":6.0}}`),
}

expr, err := govaluate.NewEvaluableExpression("hello.foo < hello.bar")
Expand All @@ -282,7 +284,7 @@ func TestMarshalUnmarshal(t *testing.T) {
var m MyType
err := json.Unmarshal(data, &m)
require.NoError(t, err)
assert.Equal(t, MyType{Foo: "hello", attrs: Attributes{data: []byte(`{"a":10,"b":"c"}`)}}, m)
assert.Equal(t, MyType{Foo: "hello", Attrs: []byte(`{"a":10,"b":"c"}`)}, m)
b, err := json.Marshal(&m)
require.NoError(t, err)
assert.Equal(t, data, b)
Expand All @@ -305,3 +307,12 @@ func BenchmarkMarshal(b *testing.B) {
json.Marshal(&m)
}
}

func TestNoLookupAttrsDirectly(t *testing.T) {
m := MyType{
Attrs: []byte(`{}`),
}
_, err := m.Get("Attrs")
require.NotNil(t, err)
assert.Equal(t, err.Error(), "[Attrs] not found")
}

0 comments on commit 6a3137d

Please sign in to comment.