Skip to content

Commit

Permalink
馃悰 fix: field set value failed for some cases
Browse files Browse the repository at this point in the history
  • Loading branch information
0xE8551CCB committed Oct 12, 2019
1 parent d436a1f commit adf3ca3
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 21 deletions.
3 changes: 0 additions & 3 deletions chell.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,6 @@ func (c *Chell) dumpField(ctx context.Context, field *schemaField, value interfa
return nil
}

if convertible(value, field.Field.Value()) {
return field.setValue(value)
}
if !field.isNested() {
logger.Debugf("[portal.chell] dump normal field %s with value %v", field, value)
return field.setValue(value)
Expand Down
17 changes: 16 additions & 1 deletion convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,25 @@ func convert(to, from interface{}) (out interface{}, err error) {
func convertWithReflect(to interface{}, from interface{}) (interface{}, error) {
expectedType := reflect.TypeOf(to)
value := reflect.ValueOf(from)

// type -> type
if value.Type().ConvertibleTo(expectedType) {
return value.Convert(expectedType).Interface(), nil
}
return nil, fmt.Errorf("failed to convert from type '%s' to '%s'", value.Type().Name(), expectedType.Name())

// type -> *type
if expectedType.Kind() == reflect.Ptr && value.Type().ConvertibleTo(expectedType.Elem()) {
expectedValue := reflect.New(expectedType.Elem())
expectedValue.Elem().Set(value.Convert(expectedType.Elem()))
return expectedValue.Interface(), nil
}

// *type -> type
if value.Type().Kind() == reflect.Ptr && value.Type().Elem().ConvertibleTo(expectedType) {
return value.Elem().Convert(expectedType).Interface(), nil
}

return nil, fmt.Errorf("failed to convert from type '%s' to '%s'", value.Type().String(), expectedType.String())
}

func toIntPtrE(v interface{}) (*int, error) {
Expand Down
53 changes: 50 additions & 3 deletions convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"

"github.com/stretchr/testify/suite"
)

Expand Down Expand Up @@ -776,9 +778,20 @@ func (s *SuiteConvertTester) Test_ConvertWithReflect() {
s.Nil(err)
s.Equal(user, out.(User))

var targetPtr *User
_, err = convert(targetPtr, user)
s.NotNil(err)
// User -> *User
out, err = convert(&target, user)
s.Nil(err)
s.Equal(user, *out.(*User))

// *User -> *User
out, err = convert(&target, &user)
s.Nil(err)
s.Equal(user, *out.(*User))

// *User -> User
out, err = convert(target, &user)
s.Nil(err)
s.Equal(user, out.(User))

var targetInt int
_, err = convert(targetInt, "1.23abc")
Expand All @@ -788,3 +801,37 @@ func (s *SuiteConvertTester) Test_ConvertWithReflect() {
func TestSuiteConvert(t *testing.T) {
suite.Run(t, new(SuiteConvertTester))
}

func Test_convertWithReflect(t *testing.T) {
var dst int
src := 10

// type -> type
ret, err := convertWithReflect(dst, src)
assert.Nil(t, err)
assert.Equal(t, 10, ret)

// *type -> *type
ret, err = convertWithReflect(&dst, &src)
assert.Nil(t, err)
assert.Equal(t, 10, *ret.(*int))

// *type -> type
ret, err = convertWithReflect(dst, &src)
assert.Nil(t, err)
assert.Equal(t, 10, ret.(int))

// type -> *type
ret, err = convertWithReflect(&dst, src)
assert.Nil(t, err)
assert.Equal(t, 10, *ret.(*int))

var dst1 int64
ret, err = convertWithReflect(dst1, src)
assert.Nil(t, err)
assert.Equal(t, int64(10), ret.(int64))

ret, err = convertWithReflect(&dst1, src)
assert.Nil(t, err)
assert.Equal(t, int64(10), *ret.(*int64))
}
38 changes: 26 additions & 12 deletions field.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"strings"
"sync"

"github.com/pkg/errors"

"reflect"

"github.com/fatih/structs"
Expand Down Expand Up @@ -51,34 +53,46 @@ func (f *schemaField) String() string {
return f.schema.name() + "." + f.Name()
}

// cases:
// value -> value
// value -> *value
// *value -> *value
// *value -> value
// Valuer -> value
// Valuer -> *value
// Valuer -> SetValuer
// value -> SetValuer
func (f *schemaField) setValue(v interface{}) error {
realValue, err := f.realInputValue(v)
if err != nil {
return err
convertedValue, err := convert(f.Value(), v)
if err == nil {
return f.Set(convertedValue)
}

convertedValue, err := convert(f.Value(), realValue)
indirectValue, err := f.inputValueIndirectly(v)
if err != nil {
return f.setIndirectly(realValue)
} else {
return errors.WithStack(err)
}

convertedValue, err = convert(f.Value(), indirectValue)
if err == nil {
return f.Set(convertedValue)
}

return f.setIndirectly(indirectValue)
}

func (f *schemaField) realInputValue(v interface{}) (interface{}, error) {
func (f *schemaField) inputValueIndirectly(v interface{}) (interface{}, error) {
var iv interface{}
rv := reflect.ValueOf(v)
switch rv.Kind() {
case reflect.Ptr:
iv = v
case reflect.Struct:
// input is `SomeStruct`
// but `*SomeStruct` implements `Valuer` interface.
default:
// input is `SomeType`
// but `*SomeType` implements `Valuer` interface.
tmpValue := reflect.New(rv.Type())
tmpValue.Elem().Set(rv)
iv = tmpValue.Interface()
default:
return v, nil
}

switch r := iv.(type) {
Expand Down
16 changes: 15 additions & 1 deletion field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"
"time"

"github.com/pkg/errors"

"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -213,6 +215,12 @@ func (t *Timestamp) Value() (interface{}, error) {
return t.tm, nil
}

type ErrorValue int

func (v ErrorValue) Value() (interface{}, error) {
return 0, errors.New("error value")
}

func TestField_SetValue(t *testing.T) {
type BarSchema struct {
ID string
Expand Down Expand Up @@ -244,7 +252,7 @@ func TestField_SetValue(t *testing.T) {
assert.Equal(t, Timestamp{now}, *f.Value().(*Timestamp))

f = newField(schema, schema.innerStruct().Field("Ts2"))
assert.Nil(t, f.setValue(Timestamp{now}))
assert.Nil(t, f.setValue(&Timestamp{now}))
assert.Equal(t, Timestamp{now}, f.Value().(Timestamp))

f = newField(schema, schema.innerStruct().Field("Ts2"))
Expand All @@ -254,6 +262,12 @@ func TestField_SetValue(t *testing.T) {
f = newField(schema, schema.innerStruct().Field("Ts2"))
assert.Nil(t, f.setValue(now))
assert.Equal(t, Timestamp{now}, f.Value().(Timestamp))

f = newField(schema, schema.innerStruct().Field("Ts2"))
assert.NotNil(t, f.setValue(100))

f = newField(schema, schema.innerStruct().Field("Ts2"))
assert.NotNil(t, f.setValue(ErrorValue(0)))
}

// BenchmarkNewField-4 3622506 317 ns/op
Expand Down
3 changes: 2 additions & 1 deletion utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,10 @@ func structName(v interface{}) string {
}

func isNil(in interface{}) bool {
if in == nil {
if in == nil || in == interface{}(nil) {
return true
}

v := reflect.ValueOf(in)
switch v.Kind() {
case reflect.Ptr, reflect.Interface, reflect.Map, reflect.Slice:
Expand Down

0 comments on commit adf3ca3

Please sign in to comment.