From 9bc94d8c700dc4dbd131e4f5311780d637447de0 Mon Sep 17 00:00:00 2001 From: Markus Ritberger Date: Wed, 14 Mar 2018 20:52:27 +0100 Subject: [PATCH 1/3] replace public function with custom error type --- request.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/request.go b/request.go index 6e14c12..2c66c69 100644 --- a/request.go +++ b/request.go @@ -29,22 +29,33 @@ var ( ErrUnknownFieldNumberType = errors.New("The struct field was not of a known number type") // ErrInvalidType is returned when the given type is incompatible with the expected type. ErrInvalidType = errors.New("Invalid type provided") // I wish we used punctuation. + ) // ErrUnsupportedPtrType is returned when the Struct field was a pointer but // the JSON value was of a different type -func ErrUnsupportedPtrType(rf reflect.Value, t reflect.Type, structField reflect.StructField) error { - typeName := t.Elem().Name() - kind := t.Elem().Kind() +type ErrUnsupportedPtrType struct { + rf reflect.Value + t reflect.Type + structField reflect.StructField +} + +func (eupt ErrUnsupportedPtrType) Error() string { + typeName := eupt.t.Elem().Name() + kind := eupt.t.Elem().Kind() if kind.String() != "" && kind.String() != typeName { typeName = fmt.Sprintf("%s (%s)", typeName, kind.String()) } - return fmt.Errorf( + return fmt.Sprintf( "jsonapi: Can't unmarshal %+v (%s) to struct field `%s`, which is a pointer to `%s`", - rf, rf.Type().Kind(), structField.Name, typeName, + eupt.rf, eupt.rf.Type().Kind(), eupt.structField.Name, typeName, ) } +func newErrUnsupportedPtrType(rf reflect.Value, t reflect.Type, structField reflect.StructField) error { + return ErrUnsupportedPtrType{rf, t, structField} +} + // UnmarshalPayload converts an io into a struct instance using jsonapi tags on // struct fields. This method supports single request payloads only, at the // moment. Bulk creates and updates are not supported yet. @@ -559,15 +570,15 @@ func handlePointer(attribute interface{}, args []string, fieldType reflect.Type, var err error concreteVal, err = handleStruct(attribute, args, fieldType, fieldValue) if err != nil { - return reflect.Value{}, ErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) + return reflect.Value{}, newErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) } return concreteVal.Elem(), err default: - return reflect.Value{}, ErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) + return reflect.Value{}, newErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) } if t != concreteVal.Type() { - return reflect.Value{}, ErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) + return reflect.Value{}, newErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) } return concreteVal, nil From 72f7bad5b3662d9687cb3e4e3b2e085ac6761c5a Mon Sep 17 00:00:00 2001 From: Markus Ritberger Date: Wed, 14 Mar 2018 21:43:24 +0100 Subject: [PATCH 2/3] check for ptr error type in tests --- request_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/request_test.go b/request_test.go index 79ec030..8ee4a5a 100644 --- a/request_test.go +++ b/request_test.go @@ -136,6 +136,9 @@ func TestUnmarshalToStructWithPointerAttr_BadType_bool(t *testing.T) { if err.Error() != expectedErrorMessage { t.Fatalf("Unexpected error message: %s", err.Error()) } + if _, ok := err.(ErrUnsupportedPtrType); !ok { + t.Fatalf("Unexpected error type: %s", reflect.TypeOf(err)) + } } func TestUnmarshalToStructWithPointerAttr_BadType_MapPtr(t *testing.T) { @@ -153,6 +156,9 @@ func TestUnmarshalToStructWithPointerAttr_BadType_MapPtr(t *testing.T) { if err.Error() != expectedErrorMessage { t.Fatalf("Unexpected error message: %s", err.Error()) } + if _, ok := err.(ErrUnsupportedPtrType); !ok { + t.Fatalf("Unexpected error type: %s", reflect.TypeOf(err)) + } } func TestUnmarshalToStructWithPointerAttr_BadType_Struct(t *testing.T) { @@ -171,6 +177,9 @@ func TestUnmarshalToStructWithPointerAttr_BadType_Struct(t *testing.T) { if err.Error() != expectedErrorMessage { t.Fatalf("Unexpected error message: %s", err.Error()) } + if _, ok := err.(ErrUnsupportedPtrType); !ok { + t.Fatalf("Unexpected error type: %s", reflect.TypeOf(err)) + } } func TestUnmarshalToStructWithPointerAttr_BadType_IntSlice(t *testing.T) { @@ -189,6 +198,9 @@ func TestUnmarshalToStructWithPointerAttr_BadType_IntSlice(t *testing.T) { if err.Error() != expectedErrorMessage { t.Fatalf("Unexpected error message: %s", err.Error()) } + if _, ok := err.(ErrUnsupportedPtrType); !ok { + t.Fatalf("Unexpected error type: %s", reflect.TypeOf(err)) + } } func TestStringPointerField(t *testing.T) { From d490a0f637974f2f7a94d10911b8f62837d68cbe Mon Sep 17 00:00:00 2001 From: Markus Ritberger Date: Wed, 14 Mar 2018 21:43:51 +0100 Subject: [PATCH 3/3] remove whitespaces and stick to 80chars --- request.go | 83 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/request.go b/request.go index 2c66c69..ff426ce 100644 --- a/request.go +++ b/request.go @@ -140,7 +140,6 @@ func UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) { } func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) (err error) { - defer func() { if r := recover(); r != nil { err = fmt.Errorf("data is not a jsonapi representation of '%v'", model.Type()) @@ -386,8 +385,11 @@ func assign(field, value reflect.Value) { } } -func unmarshalAttribute(attribute interface{}, args []string, structField reflect.StructField, fieldValue reflect.Value) (value reflect.Value, err error) { - +func unmarshalAttribute( + attribute interface{}, + args []string, + structField reflect.StructField, + fieldValue reflect.Value) (value reflect.Value, err error) { value = reflect.ValueOf(attribute) fieldType := structField.Type @@ -398,7 +400,8 @@ func unmarshalAttribute(attribute interface{}, args []string, structField reflec } // Handle field of type time.Time - if fieldValue.Type() == reflect.TypeOf(time.Time{}) || fieldValue.Type() == reflect.TypeOf(new(time.Time)) { + if fieldValue.Type() == reflect.TypeOf(time.Time{}) || + fieldValue.Type() == reflect.TypeOf(new(time.Time)) { value, err = handleTime(attribute, args, fieldType, fieldValue) return } @@ -410,7 +413,8 @@ func unmarshalAttribute(attribute interface{}, args []string, structField reflec } // Handle field containing slice of structs - if fieldValue.Type().Kind() == reflect.Slice && reflect.TypeOf(fieldValue.Interface()).Elem().Kind() == reflect.Struct { + if fieldValue.Type().Kind() == reflect.Slice && + reflect.TypeOf(fieldValue.Interface()).Elem().Kind() == reflect.Struct { value, err = handleStructSlice(attribute, args, fieldType, fieldValue) return } @@ -436,7 +440,11 @@ func unmarshalAttribute(attribute interface{}, args []string, structField reflec return } -func handleStringSlice(attribute interface{}, args []string, fieldType reflect.Type, fieldValue reflect.Value) (reflect.Value, error) { +func handleStringSlice( + attribute interface{}, + args []string, + fieldType reflect.Type, + fieldValue reflect.Value) (reflect.Value, error) { v := reflect.ValueOf(attribute) values := make([]string, v.Len()) for i := 0; i < v.Len(); i++ { @@ -446,8 +454,11 @@ func handleStringSlice(attribute interface{}, args []string, fieldType reflect.T return reflect.ValueOf(values), nil } -func handleTime(attribute interface{}, args []string, fieldType reflect.Type, fieldValue reflect.Value) (reflect.Value, error) { - +func handleTime( + attribute interface{}, + args []string, + fieldType reflect.Type, + fieldValue reflect.Value) (reflect.Value, error) { var isIso8601 bool v := reflect.ValueOf(attribute) @@ -494,7 +505,11 @@ func handleTime(attribute interface{}, args []string, fieldType reflect.Type, fi return reflect.ValueOf(t), nil } -func handleNumeric(attribute interface{}, args []string, fieldType reflect.Type, fieldValue reflect.Value) (reflect.Value, error) { +func handleNumeric( + attribute interface{}, + args []string, + fieldType reflect.Type, + fieldValue reflect.Value) (reflect.Value, error) { v := reflect.ValueOf(attribute) floatValue := v.Interface().(float64) @@ -551,7 +566,12 @@ func handleNumeric(attribute interface{}, args []string, fieldType reflect.Type, return numericValue, nil } -func handlePointer(attribute interface{}, args []string, fieldType reflect.Type, fieldValue reflect.Value, structField reflect.StructField) (reflect.Value, error) { +func handlePointer( + attribute interface{}, + args []string, + fieldType reflect.Type, + fieldValue reflect.Value, + structField reflect.StructField) (reflect.Value, error) { t := fieldValue.Type() var concreteVal reflect.Value @@ -560,50 +580,55 @@ func handlePointer(attribute interface{}, args []string, fieldType reflect.Type, concreteVal = reflect.ValueOf(&cVal) case bool: concreteVal = reflect.ValueOf(&cVal) - case complex64: - concreteVal = reflect.ValueOf(&cVal) - case complex128: - concreteVal = reflect.ValueOf(&cVal) - case uintptr: + case complex64, complex128, uintptr: concreteVal = reflect.ValueOf(&cVal) case map[string]interface{}: var err error concreteVal, err = handleStruct(attribute, args, fieldType, fieldValue) if err != nil { - return reflect.Value{}, newErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) + return reflect.Value{}, newErrUnsupportedPtrType( + reflect.ValueOf(attribute), fieldType, structField) } return concreteVal.Elem(), err default: - return reflect.Value{}, newErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) + return reflect.Value{}, newErrUnsupportedPtrType( + reflect.ValueOf(attribute), fieldType, structField) } if t != concreteVal.Type() { - return reflect.Value{}, newErrUnsupportedPtrType(reflect.ValueOf(attribute), fieldType, structField) + return reflect.Value{}, newErrUnsupportedPtrType( + reflect.ValueOf(attribute), fieldType, structField) } return concreteVal, nil } -func handleStruct(attribute interface{}, args []string, fieldType reflect.Type, fieldValue reflect.Value) (reflect.Value, error) { +func handleStruct( + attribute interface{}, + args []string, + fieldType reflect.Type, + fieldValue reflect.Value) (reflect.Value, error) { model := reflect.New(fieldValue.Type()) - var er error - - data, er := json.Marshal(attribute) - if er != nil { - return model, er + data, err := json.Marshal(attribute) + if err != nil { + return model, err } - er = json.Unmarshal(data, model.Interface()) + err = json.Unmarshal(data, model.Interface()) - if er != nil { - return model, er + if err != nil { + return model, err } - return model, er + return model, err } -func handleStructSlice(attribute interface{}, args []string, fieldType reflect.Type, fieldValue reflect.Value) (reflect.Value, error) { +func handleStructSlice( + attribute interface{}, + args []string, + fieldType reflect.Type, + fieldValue reflect.Value) (reflect.Value, error) { models := reflect.New(fieldValue.Type()).Elem() dataMap := reflect.ValueOf(attribute).Interface().([]interface{}) for _, data := range dataMap {