From 53197ad312f527a84ef05a05703df34584cd4a77 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 3 Feb 2017 17:53:23 -0800 Subject: [PATCH 01/15] working version --- constants.go | 1 + node.go | 32 ++++++ request.go | 18 +++- response.go | 29 ++++++ response_embedded_test.go | 205 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 response_embedded_test.go diff --git a/constants.go b/constants.go index 23288d31..d5cd0ea5 100644 --- a/constants.go +++ b/constants.go @@ -10,6 +10,7 @@ const ( annotationOmitEmpty = "omitempty" annotationISO8601 = "iso8601" annotationSeperator = "," + annotationIgnore = "-" iso8601TimeFormat = "2006-01-02T15:04:05Z" diff --git a/node.go b/node.go index a58488c8..39bf0d29 100644 --- a/node.go +++ b/node.go @@ -44,6 +44,38 @@ type Node struct { Meta *Meta `json:"meta,omitempty"` } +func (n *Node) merge(node *Node) { + if node.Type != "" { + n.Type = node.Type + } + + if node.ID != "" { + n.ID = node.ID + } + + if node.ClientID != "" { + n.ClientID = node.ClientID + } + + if n.Attributes == nil && node.Attributes != nil { + n.Attributes = make(map[string]interface{}) + } + for k, v := range node.Attributes { + n.Attributes[k] = v + } + + if n.Relationships == nil && n.Relationships != nil { + n.Relationships = make(map[string]interface{}) + } + for k, v := range node.Relationships { + n.Relationships[k] = v + } + + if node.Links != nil { + n.Links = node.Links + } +} + // RelationshipOneNode is used to represent a generic has one JSON API relation type RelationshipOneNode struct { Data *Node `json:"data"` diff --git a/request.go b/request.go index fe29706a..0914a016 100644 --- a/request.go +++ b/request.go @@ -131,14 +131,28 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) for i := 0; i < modelValue.NumField(); i++ { fieldType := modelType.Field(i) - tag := fieldType.Tag.Get("jsonapi") + tag := fieldType.Tag.Get(annotationJSONAPI) + + // handles embedded structs + if isEmbeddedStruct(fieldType) { + if shouldIgnoreField(tag) { + continue + } + model := reflect.ValueOf(modelValue.Field(i).Addr().Interface()) + err := unmarshalNode(data, model, included) + if err != nil { + er = err + break + } + } + if tag == "" { continue } fieldValue := modelValue.Field(i) - args := strings.Split(tag, ",") + args := strings.Split(tag, annotationSeperator) if len(args) < 1 { er = ErrBadJSONAPIStructTag diff --git a/response.go b/response.go index 76c3a3d7..ee70d086 100644 --- a/response.go +++ b/response.go @@ -202,6 +202,20 @@ func MarshalOnePayloadEmbedded(w io.Writer, model interface{}) error { return nil } +func isEmbeddedStruct(sField reflect.StructField) bool { + if sField.Anonymous && sField.Type.Kind() == reflect.Struct { + return true + } + return false +} + +func shouldIgnoreField(japiTag string) bool { + if strings.HasPrefix(japiTag, annotationIgnore) { + return true + } + return false +} + func visitModelNode(model interface{}, included *map[string]*Node, sideload bool) (*Node, error) { node := new(Node) @@ -214,6 +228,21 @@ func visitModelNode(model interface{}, included *map[string]*Node, for i := 0; i < modelValue.NumField(); i++ { structField := modelValue.Type().Field(i) tag := structField.Tag.Get(annotationJSONAPI) + + // handles embedded structs + if isEmbeddedStruct(structField) { + if shouldIgnoreField(tag) { + continue + } + model := modelValue.Field(i).Addr().Interface() + embNode, err := visitModelNode(model, included, sideload) + if err != nil { + er = err + break + } + node.merge(embNode) + } + if tag == "" { continue } diff --git a/response_embedded_test.go b/response_embedded_test.go new file mode 100644 index 00000000..51e3bf05 --- /dev/null +++ b/response_embedded_test.go @@ -0,0 +1,205 @@ +package jsonapi + +import ( + "bytes" + "reflect" + "testing" +) + +func TestMergeNode(t *testing.T) { + parent := &Node{ + Type: "Good", + ID: "99", + Attributes: map[string]interface{}{"fizz": "buzz"}, + } + + child := &Node{ + Type: "Better", + ClientID: "1111", + Attributes: map[string]interface{}{"timbuk": 2}, + } + + expected := &Node{ + Type: "Better", + ID: "99", + ClientID: "1111", + Attributes: map[string]interface{}{"fizz": "buzz", "timbuk": 2}, + } + + parent.merge(child) + + if !reflect.DeepEqual(expected, parent) { + t.Errorf("Got %+v Expected %+v", parent, expected) + } +} + +func TestIsEmbeddedStruct(t *testing.T) { + type foo struct{} + + structType := reflect.TypeOf(foo{}) + stringType := reflect.TypeOf("") + if structType.Kind() != reflect.Struct { + t.Fatal("structType.Kind() is not a struct.") + } + if stringType.Kind() != reflect.String { + t.Fatal("stringType.Kind() is not a string.") + } + + type test struct { + scenario string + input reflect.StructField + expectedRes bool + } + + tests := []test{ + test{ + scenario: "success", + input: reflect.StructField{Anonymous: true, Type: structType}, + expectedRes: true, + }, + test{ + scenario: "wrong type", + input: reflect.StructField{Anonymous: true, Type: stringType}, + expectedRes: false, + }, + test{ + scenario: "not embedded", + input: reflect.StructField{Type: structType}, + expectedRes: false, + }, + } + + for _, test := range tests { + res := isEmbeddedStruct(test.input) + if res != test.expectedRes { + t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) + } + } +} + +func TestShouldIgnoreField(t *testing.T) { + type test struct { + scenario string + input string + expectedRes bool + } + + tests := []test{ + test{ + scenario: "opt-out", + input: annotationIgnore, + expectedRes: true, + }, + test{ + scenario: "no tag", + input: "", + expectedRes: false, + }, + test{ + scenario: "wrong tag", + input: "wrong,tag", + expectedRes: false, + }, + } + + for _, test := range tests { + res := shouldIgnoreField(test.input) + if res != test.expectedRes { + t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) + } + } +} + +func TestIsValidEmbeddedStruct(t *testing.T) { + type foo struct{} + + structType := reflect.TypeOf(foo{}) + stringType := reflect.TypeOf("") + if structType.Kind() != reflect.Struct { + t.Fatal("structType.Kind() is not a struct.") + } + if stringType.Kind() != reflect.String { + t.Fatal("stringType.Kind() is not a string.") + } + + type test struct { + scenario string + input reflect.StructField + expectedRes bool + } + + tests := []test{ + test{ + scenario: "success", + input: reflect.StructField{Anonymous: true, Type: structType}, + expectedRes: true, + }, + test{ + scenario: "opt-out", + input: reflect.StructField{Anonymous: true, Tag: "jsonapi:\"-\"", Type: structType}, + expectedRes: false, + }, + test{ + scenario: "wrong type", + input: reflect.StructField{Anonymous: true, Type: stringType}, + expectedRes: false, + }, + test{ + scenario: "not embedded", + input: reflect.StructField{Type: structType}, + expectedRes: false, + }, + } + + for _, test := range tests { + res := (isEmbeddedStruct(test.input) && !shouldIgnoreField(test.input.Tag.Get(annotationJSONAPI))) + if res != test.expectedRes { + t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) + } + } +} + +func TestMarshalUnmarshalCompositeStruct(t *testing.T) { + type Thing struct { + ID int `jsonapi:"primary,things"` + Fizz string `jsonapi:"attr,fizz"` + Buzz int `jsonapi:"attr,buzz"` + } + + type Model struct { + Thing + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + } + + model := &Model{} + model.ID = 1 + model.Fizz = "fizzy" + model.Buzz = 99 + model.Foo = "fooey" + model.Bar = "barry" + model.Bat = "batty" + + buf := bytes.NewBuffer(nil) + if err := MarshalOnePayload(buf, model); err != nil { + t.Fatal(err) + } + + // TODO: redo this + // assert encoding from model to jsonapi output + // expected := `{"data":{"type":"things","id":"1","attributes":{"bar":"barry","bat":"batty","buzz":99,"fizz":"fizzy","foo":"fooey"}}}` + // if expected != string(buf.Bytes()) { + // t.Errorf("Got %+v Expected %+v", string(buf.Bytes()), expected) + // } + + dst := &Model{} + if err := UnmarshalPayload(buf, dst); err != nil { + t.Fatal(err) + } + + // assert decoding from jsonapi output to model + if !reflect.DeepEqual(model, dst) { + t.Errorf("Got %#v Expected %#v", dst, model) + } +} From 9045ea96b0529359cd972b54d8070ff6ded42a6e Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Mon, 6 Feb 2017 09:42:47 -0800 Subject: [PATCH 02/15] fix text --- response_embedded_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/response_embedded_test.go b/response_embedded_test.go index 51e3bf05..0d997ca4 100644 --- a/response_embedded_test.go +++ b/response_embedded_test.go @@ -3,6 +3,7 @@ package jsonapi import ( "bytes" "reflect" + "strings" "testing" ) @@ -186,12 +187,13 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { t.Fatal(err) } - // TODO: redo this // assert encoding from model to jsonapi output - // expected := `{"data":{"type":"things","id":"1","attributes":{"bar":"barry","bat":"batty","buzz":99,"fizz":"fizzy","foo":"fooey"}}}` - // if expected != string(buf.Bytes()) { - // t.Errorf("Got %+v Expected %+v", string(buf.Bytes()), expected) - // } + expected := `{"data":{"type":"things","id":"1","attributes":{"bar":"barry","bat":"batty","buzz":99,"fizz":"fizzy","foo":"fooey"}}}` + actual := strings.TrimSpace(string(buf.Bytes())) + + if expected != actual { + t.Errorf("Got %+v Expected %+v", actual, expected) + } dst := &Model{} if err := UnmarshalPayload(buf, dst); err != nil { From 7bb11b80c7d0f52a84534c339a9f62f30882f5ec Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Mon, 6 Feb 2017 10:15:32 -0800 Subject: [PATCH 03/15] combine test files --- response_embedded_test.go | 207 -------------------------------------- response_test.go | 200 ++++++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 207 deletions(-) delete mode 100644 response_embedded_test.go diff --git a/response_embedded_test.go b/response_embedded_test.go deleted file mode 100644 index 0d997ca4..00000000 --- a/response_embedded_test.go +++ /dev/null @@ -1,207 +0,0 @@ -package jsonapi - -import ( - "bytes" - "reflect" - "strings" - "testing" -) - -func TestMergeNode(t *testing.T) { - parent := &Node{ - Type: "Good", - ID: "99", - Attributes: map[string]interface{}{"fizz": "buzz"}, - } - - child := &Node{ - Type: "Better", - ClientID: "1111", - Attributes: map[string]interface{}{"timbuk": 2}, - } - - expected := &Node{ - Type: "Better", - ID: "99", - ClientID: "1111", - Attributes: map[string]interface{}{"fizz": "buzz", "timbuk": 2}, - } - - parent.merge(child) - - if !reflect.DeepEqual(expected, parent) { - t.Errorf("Got %+v Expected %+v", parent, expected) - } -} - -func TestIsEmbeddedStruct(t *testing.T) { - type foo struct{} - - structType := reflect.TypeOf(foo{}) - stringType := reflect.TypeOf("") - if structType.Kind() != reflect.Struct { - t.Fatal("structType.Kind() is not a struct.") - } - if stringType.Kind() != reflect.String { - t.Fatal("stringType.Kind() is not a string.") - } - - type test struct { - scenario string - input reflect.StructField - expectedRes bool - } - - tests := []test{ - test{ - scenario: "success", - input: reflect.StructField{Anonymous: true, Type: structType}, - expectedRes: true, - }, - test{ - scenario: "wrong type", - input: reflect.StructField{Anonymous: true, Type: stringType}, - expectedRes: false, - }, - test{ - scenario: "not embedded", - input: reflect.StructField{Type: structType}, - expectedRes: false, - }, - } - - for _, test := range tests { - res := isEmbeddedStruct(test.input) - if res != test.expectedRes { - t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) - } - } -} - -func TestShouldIgnoreField(t *testing.T) { - type test struct { - scenario string - input string - expectedRes bool - } - - tests := []test{ - test{ - scenario: "opt-out", - input: annotationIgnore, - expectedRes: true, - }, - test{ - scenario: "no tag", - input: "", - expectedRes: false, - }, - test{ - scenario: "wrong tag", - input: "wrong,tag", - expectedRes: false, - }, - } - - for _, test := range tests { - res := shouldIgnoreField(test.input) - if res != test.expectedRes { - t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) - } - } -} - -func TestIsValidEmbeddedStruct(t *testing.T) { - type foo struct{} - - structType := reflect.TypeOf(foo{}) - stringType := reflect.TypeOf("") - if structType.Kind() != reflect.Struct { - t.Fatal("structType.Kind() is not a struct.") - } - if stringType.Kind() != reflect.String { - t.Fatal("stringType.Kind() is not a string.") - } - - type test struct { - scenario string - input reflect.StructField - expectedRes bool - } - - tests := []test{ - test{ - scenario: "success", - input: reflect.StructField{Anonymous: true, Type: structType}, - expectedRes: true, - }, - test{ - scenario: "opt-out", - input: reflect.StructField{Anonymous: true, Tag: "jsonapi:\"-\"", Type: structType}, - expectedRes: false, - }, - test{ - scenario: "wrong type", - input: reflect.StructField{Anonymous: true, Type: stringType}, - expectedRes: false, - }, - test{ - scenario: "not embedded", - input: reflect.StructField{Type: structType}, - expectedRes: false, - }, - } - - for _, test := range tests { - res := (isEmbeddedStruct(test.input) && !shouldIgnoreField(test.input.Tag.Get(annotationJSONAPI))) - if res != test.expectedRes { - t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) - } - } -} - -func TestMarshalUnmarshalCompositeStruct(t *testing.T) { - type Thing struct { - ID int `jsonapi:"primary,things"` - Fizz string `jsonapi:"attr,fizz"` - Buzz int `jsonapi:"attr,buzz"` - } - - type Model struct { - Thing - Foo string `jsonapi:"attr,foo"` - Bar string `jsonapi:"attr,bar"` - Bat string `jsonapi:"attr,bat"` - } - - model := &Model{} - model.ID = 1 - model.Fizz = "fizzy" - model.Buzz = 99 - model.Foo = "fooey" - model.Bar = "barry" - model.Bat = "batty" - - buf := bytes.NewBuffer(nil) - if err := MarshalOnePayload(buf, model); err != nil { - t.Fatal(err) - } - - // assert encoding from model to jsonapi output - expected := `{"data":{"type":"things","id":"1","attributes":{"bar":"barry","bat":"batty","buzz":99,"fizz":"fizzy","foo":"fooey"}}}` - actual := strings.TrimSpace(string(buf.Bytes())) - - if expected != actual { - t.Errorf("Got %+v Expected %+v", actual, expected) - } - - dst := &Model{} - if err := UnmarshalPayload(buf, dst); err != nil { - t.Fatal(err) - } - - // assert decoding from jsonapi output to model - if !reflect.DeepEqual(model, dst) { - t.Errorf("Got %#v Expected %#v", dst, model) - } -} diff --git a/response_test.go b/response_test.go index 71589dc7..9fa98651 100644 --- a/response_test.go +++ b/response_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "reflect" "sort" + "strings" "testing" "time" ) @@ -817,6 +818,205 @@ func TestMarshal_InvalidIntefaceArgument(t *testing.T) { } } +func TestMergeNode(t *testing.T) { + parent := &Node{ + Type: "Good", + ID: "99", + Attributes: map[string]interface{}{"fizz": "buzz"}, + } + + child := &Node{ + Type: "Better", + ClientID: "1111", + Attributes: map[string]interface{}{"timbuk": 2}, + } + + expected := &Node{ + Type: "Better", + ID: "99", + ClientID: "1111", + Attributes: map[string]interface{}{"fizz": "buzz", "timbuk": 2}, + } + + parent.merge(child) + + if !reflect.DeepEqual(expected, parent) { + t.Errorf("Got %+v Expected %+v", parent, expected) + } +} + +func TestIsEmbeddedStruct(t *testing.T) { + type foo struct{} + + structType := reflect.TypeOf(foo{}) + stringType := reflect.TypeOf("") + if structType.Kind() != reflect.Struct { + t.Fatal("structType.Kind() is not a struct.") + } + if stringType.Kind() != reflect.String { + t.Fatal("stringType.Kind() is not a string.") + } + + type test struct { + scenario string + input reflect.StructField + expectedRes bool + } + + tests := []test{ + test{ + scenario: "success", + input: reflect.StructField{Anonymous: true, Type: structType}, + expectedRes: true, + }, + test{ + scenario: "wrong type", + input: reflect.StructField{Anonymous: true, Type: stringType}, + expectedRes: false, + }, + test{ + scenario: "not embedded", + input: reflect.StructField{Type: structType}, + expectedRes: false, + }, + } + + for _, test := range tests { + res := isEmbeddedStruct(test.input) + if res != test.expectedRes { + t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) + } + } +} + +func TestShouldIgnoreField(t *testing.T) { + type test struct { + scenario string + input string + expectedRes bool + } + + tests := []test{ + test{ + scenario: "opt-out", + input: annotationIgnore, + expectedRes: true, + }, + test{ + scenario: "no tag", + input: "", + expectedRes: false, + }, + test{ + scenario: "wrong tag", + input: "wrong,tag", + expectedRes: false, + }, + } + + for _, test := range tests { + res := shouldIgnoreField(test.input) + if res != test.expectedRes { + t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) + } + } +} + +func TestIsValidEmbeddedStruct(t *testing.T) { + type foo struct{} + + structType := reflect.TypeOf(foo{}) + stringType := reflect.TypeOf("") + if structType.Kind() != reflect.Struct { + t.Fatal("structType.Kind() is not a struct.") + } + if stringType.Kind() != reflect.String { + t.Fatal("stringType.Kind() is not a string.") + } + + type test struct { + scenario string + input reflect.StructField + expectedRes bool + } + + tests := []test{ + test{ + scenario: "success", + input: reflect.StructField{Anonymous: true, Type: structType}, + expectedRes: true, + }, + test{ + scenario: "opt-out", + input: reflect.StructField{Anonymous: true, Tag: "jsonapi:\"-\"", Type: structType}, + expectedRes: false, + }, + test{ + scenario: "wrong type", + input: reflect.StructField{Anonymous: true, Type: stringType}, + expectedRes: false, + }, + test{ + scenario: "not embedded", + input: reflect.StructField{Type: structType}, + expectedRes: false, + }, + } + + for _, test := range tests { + res := (isEmbeddedStruct(test.input) && !shouldIgnoreField(test.input.Tag.Get(annotationJSONAPI))) + if res != test.expectedRes { + t.Errorf("Scenario -> %s\nGot -> %v\nExpected -> %v\n", test.scenario, res, test.expectedRes) + } + } +} + +func TestMarshalUnmarshalCompositeStruct(t *testing.T) { + type Thing struct { + ID int `jsonapi:"primary,things"` + Fizz string `jsonapi:"attr,fizz"` + Buzz int `jsonapi:"attr,buzz"` + } + + type Model struct { + Thing + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + } + + model := &Model{} + model.ID = 1 + model.Fizz = "fizzy" + model.Buzz = 99 + model.Foo = "fooey" + model.Bar = "barry" + model.Bat = "batty" + + buf := bytes.NewBuffer(nil) + if err := MarshalOnePayload(buf, model); err != nil { + t.Fatal(err) + } + + // assert encoding from model to jsonapi output + expected := `{"data":{"type":"things","id":"1","attributes":{"bar":"barry","bat":"batty","buzz":99,"fizz":"fizzy","foo":"fooey"}}}` + actual := strings.TrimSpace(string(buf.Bytes())) + + if expected != actual { + t.Errorf("Got %+v Expected %+v", actual, expected) + } + + dst := &Model{} + if err := UnmarshalPayload(buf, dst); err != nil { + t.Fatal(err) + } + + // assert decoding from jsonapi output to model + if !reflect.DeepEqual(model, dst) { + t.Errorf("Got %#v Expected %#v", dst, model) + } +} + func testBlog() *Blog { return &Blog{ ID: 5, From 87fcc79e5670d13a7caa7c19d6ef6af440f949cb Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Mon, 6 Feb 2017 10:18:56 -0800 Subject: [PATCH 04/15] move private funcs to bottom --- response.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/response.go b/response.go index ee70d086..0824fcf4 100644 --- a/response.go +++ b/response.go @@ -202,20 +202,6 @@ func MarshalOnePayloadEmbedded(w io.Writer, model interface{}) error { return nil } -func isEmbeddedStruct(sField reflect.StructField) bool { - if sField.Anonymous && sField.Type.Kind() == reflect.Struct { - return true - } - return false -} - -func shouldIgnoreField(japiTag string) bool { - if strings.HasPrefix(japiTag, annotationIgnore) { - return true - } - return false -} - func visitModelNode(model interface{}, included *map[string]*Node, sideload bool) (*Node, error) { node := new(Node) @@ -562,3 +548,17 @@ func convertToSliceInterface(i *interface{}) ([]interface{}, error) { } return response, nil } + +func isEmbeddedStruct(sField reflect.StructField) bool { + if sField.Anonymous && sField.Type.Kind() == reflect.Struct { + return true + } + return false +} + +func shouldIgnoreField(japiTag string) bool { + if strings.HasPrefix(japiTag, annotationIgnore) { + return true + } + return false +} From 1b5f1b494986b300f981b15113b532dfcc86eac7 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Mon, 20 Feb 2017 21:06:04 -0800 Subject: [PATCH 05/15] ErrInvalidType should ignore interfaces --- request.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/request.go b/request.go index 0914a016..05bed443 100644 --- a/request.go +++ b/request.go @@ -460,7 +460,8 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) } // As a final catch-all, ensure types line up to avoid a runtime panic. - if fieldValue.Kind() != v.Kind() { + // Ignore interfaces since interfaces are poly + if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != v.Kind() { return ErrInvalidType } fieldValue.Set(reflect.ValueOf(val)) From 06cdde66893fda44608f9741086f406493c0e400 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Tue, 18 Jul 2017 09:05:12 -0700 Subject: [PATCH 06/15] replace MarshalOnePayload w/ MarshalPayload; fix bug w/ node merge() --- node.go | 2 +- response_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/node.go b/node.go index 39bf0d29..6152ba4d 100644 --- a/node.go +++ b/node.go @@ -64,7 +64,7 @@ func (n *Node) merge(node *Node) { n.Attributes[k] = v } - if n.Relationships == nil && n.Relationships != nil { + if n.Relationships == nil && node.Relationships != nil { n.Relationships = make(map[string]interface{}) } for k, v := range node.Relationships { diff --git a/response_test.go b/response_test.go index 9fa98651..aac265f5 100644 --- a/response_test.go +++ b/response_test.go @@ -994,7 +994,7 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { model.Bat = "batty" buf := bytes.NewBuffer(nil) - if err := MarshalOnePayload(buf, model); err != nil { + if err := MarshalPayload(buf, model); err != nil { t.Fatal(err) } From 01c24320eba13e0d430e30f12938d87b0446cf41 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Tue, 18 Jul 2017 09:34:33 -0700 Subject: [PATCH 07/15] minor tweaks; address a couple comments --- request.go | 6 +++--- response.go | 10 ++-------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/request.go b/request.go index 05bed443..332e426d 100644 --- a/request.go +++ b/request.go @@ -131,6 +131,8 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) for i := 0; i < modelValue.NumField(); i++ { fieldType := modelType.Field(i) + fieldValue := modelValue.Field(i) + tag := fieldType.Tag.Get(annotationJSONAPI) // handles embedded structs @@ -138,7 +140,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) if shouldIgnoreField(tag) { continue } - model := reflect.ValueOf(modelValue.Field(i).Addr().Interface()) + model := reflect.ValueOf(fieldValue.Addr().Interface()) err := unmarshalNode(data, model, included) if err != nil { er = err @@ -150,8 +152,6 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) continue } - fieldValue := modelValue.Field(i) - args := strings.Split(tag, annotationSeperator) if len(args) < 1 { diff --git a/response.go b/response.go index 0824fcf4..0f34df2b 100644 --- a/response.go +++ b/response.go @@ -550,15 +550,9 @@ func convertToSliceInterface(i *interface{}) ([]interface{}, error) { } func isEmbeddedStruct(sField reflect.StructField) bool { - if sField.Anonymous && sField.Type.Kind() == reflect.Struct { - return true - } - return false + return sField.Anonymous && sField.Type.Kind() == reflect.Struct } func shouldIgnoreField(japiTag string) bool { - if strings.HasPrefix(japiTag, annotationIgnore) { - return true - } - return false + return strings.HasPrefix(japiTag, annotationIgnore) } From ad5f5cdaf5354fcf29012b15b96d4d4992360e37 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Tue, 18 Jul 2017 15:19:00 -0700 Subject: [PATCH 08/15] decompose unmarshalNode() to smaller funcs; unmarshal should go from top-level to embedded --- request.go | 728 +++++++++++++++++++++++++---------------------- response_test.go | 175 ++++++++++-- 2 files changed, 544 insertions(+), 359 deletions(-) diff --git a/request.go b/request.go index 332e426d..29151385 100644 --- a/request.go +++ b/request.go @@ -117,6 +117,7 @@ func UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) { return models, nil } +// TODO: refactor so that it unmarshals from top to bottom func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) (err error) { defer func() { if r := recover(); r != nil { @@ -127,421 +128,476 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) modelValue := model.Elem() modelType := model.Type().Elem() - var er error + embeddedModels := []reflect.Value{} for i := 0; i < modelValue.NumField(); i++ { fieldType := modelType.Field(i) fieldValue := modelValue.Field(i) - tag := fieldType.Tag.Get(annotationJSONAPI) + // handle explicit ignore annotation + if shouldIgnoreField(tag) { + continue + } + // handles embedded structs if isEmbeddedStruct(fieldType) { - if shouldIgnoreField(tag) { - continue - } - model := reflect.ValueOf(fieldValue.Addr().Interface()) - err := unmarshalNode(data, model, included) - if err != nil { - er = err - break - } + embeddedModels = append(embeddedModels, reflect.ValueOf(fieldValue.Addr().Interface())) } + // handle tagless; after handling embedded structs (which could be tagless) if tag == "" { continue } args := strings.Split(tag, annotationSeperator) - + // require atleast 1 if len(args) < 1 { - er = ErrBadJSONAPIStructTag - break + return ErrBadJSONAPIStructTag } - annotation := args[0] - - if (annotation == annotationClientID && len(args) != 1) || - (annotation != annotationClientID && len(args) < 2) { - er = ErrBadJSONAPIStructTag - break + // annotation == args[0] + switch args[0] { + case annotationClientID: + //fmt.Printf("\nannotationClientID %v\n", args) + if err := handleClientIDUnmarshal(data, args, fieldValue); err != nil { + return err + } + case annotationPrimary: + //fmt.Printf("\nannotationPrimary %v\n", args) + if err := handlePrimaryUnmarshal(data, args, fieldType, fieldValue); err != nil { + return err + } + case annotationAttribute: + //fmt.Printf("\nannotationAttribute %v\n", args) + // fmt.Println("before", data.Attributes) + if err := handleAttributeUnmarshal(data, args, fieldType, fieldValue); err != nil { + return err + } + // fmt.Println("after ", data.Attributes) + case annotationRelation: + //fmt.Printf("\nannotationRelation %v\n", args) + if err := handleRelationUnmarshal(data, args, fieldValue, included); err != nil { + return err + } + default: + return fmt.Errorf(unsuportedStructTagMsg, args[0]) + } + } + // handle embedded last + for _, em := range embeddedModels { + if err := unmarshalNode(data, em, included); err != nil { + return err } + } - if annotation == annotationPrimary { - if data.ID == "" { - continue - } + return nil +} - // Check the JSON API Type - if data.Type != args[1] { - er = fmt.Errorf( - "Trying to Unmarshal an object of type %#v, but %#v does not match", - data.Type, - args[1], - ) - break - } +func handleClientIDUnmarshal(data *Node, args []string, fieldValue reflect.Value) error { + if len(args) != 1 { + return ErrBadJSONAPIStructTag + } - // ID will have to be transmitted as astring per the JSON API spec - v := reflect.ValueOf(data.ID) + if data.ClientID == "" { + return nil + } - // Deal with PTRS - var kind reflect.Kind - if fieldValue.Kind() == reflect.Ptr { - kind = fieldType.Type.Elem().Kind() - } else { - kind = fieldType.Type.Kind() - } + fieldValue.Set(reflect.ValueOf(data.ClientID)) - // Handle String case - if kind == reflect.String { - assign(fieldValue, v) - continue - } + // clear clientID to denote it's already been processed + data.ClientID = "" - // Value was not a string... only other supported type was a numeric, - // which would have been sent as a float value. - floatValue, err := strconv.ParseFloat(data.ID, 64) - if err != nil { - // Could not convert the value in the "id" attr to a float - er = ErrBadJSONAPIID - break - } + return nil +} - // Convert the numeric float to one of the supported ID numeric types - // (int[8,16,32,64] or uint[8,16,32,64]) - var idValue reflect.Value - switch kind { - case reflect.Int: - n := int(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int8: - n := int8(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int16: - n := int16(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int32: - n := int32(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int64: - n := int64(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint: - n := uint(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint8: - n := uint8(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint16: - n := uint16(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint32: - n := uint32(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint64: - n := uint64(floatValue) - idValue = reflect.ValueOf(&n) - default: - // We had a JSON float (numeric), but our field was not one of the - // allowed numeric types - er = ErrBadJSONAPIID - break - } +func handlePrimaryUnmarshal(data *Node, args []string, fieldType reflect.StructField, fieldValue reflect.Value) error { + if len(args) < 2 { + return ErrBadJSONAPIStructTag + } - assign(fieldValue, idValue) - } else if annotation == annotationClientID { - if data.ClientID == "" { - continue - } + if data.ID == "" { + return nil + } - fieldValue.Set(reflect.ValueOf(data.ClientID)) - } else if annotation == annotationAttribute { - attributes := data.Attributes - if attributes == nil || len(data.Attributes) == 0 { - continue - } + // Check the JSON API Type + if data.Type != args[1] { + return fmt.Errorf( + "Trying to Unmarshal an object of type %#v, but %#v does not match", + data.Type, + args[1], + ) + } - var iso8601 bool + // ID will have to be transmitted as astring per the JSON API spec + v := reflect.ValueOf(data.ID) - if len(args) > 2 { - for _, arg := range args[2:] { - if arg == annotationISO8601 { - iso8601 = true - } - } - } + // Deal with PTRS + var kind reflect.Kind + if fieldValue.Kind() == reflect.Ptr { + kind = fieldType.Type.Elem().Kind() + } else { + kind = fieldType.Type.Kind() + } - val := attributes[args[1]] + // Handle String case + if kind == reflect.String { + assign(fieldValue, v) + return nil + } - // continue if the attribute was not included in the request - if val == nil { - continue - } + // Value was not a string... only other supported type was a numeric, + // which would have been sent as a float value. + floatValue, err := strconv.ParseFloat(data.ID, 64) + if err != nil { + // Could not convert the value in the "id" attr to a float + return ErrBadJSONAPIID + } - v := reflect.ValueOf(val) + // Convert the numeric float to one of the supported ID numeric types + // (int[8,16,32,64] or uint[8,16,32,64]) + var idValue reflect.Value + switch kind { + case reflect.Int: + n := int(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int8: + n := int8(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int16: + n := int16(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int32: + n := int32(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int64: + n := int64(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint: + n := uint(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint8: + n := uint8(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint16: + n := uint16(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint32: + n := uint32(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint64: + n := uint64(floatValue) + idValue = reflect.ValueOf(&n) + default: + // We had a JSON float (numeric), but our field was not one of the + // allowed numeric types + return ErrBadJSONAPIID + } - // Handle field of type time.Time - if fieldValue.Type() == reflect.TypeOf(time.Time{}) { - if iso8601 { - var tm string - if v.Kind() == reflect.String { - tm = v.Interface().(string) - } else { - er = ErrInvalidISO8601 - break - } + assign(fieldValue, idValue) - t, err := time.Parse(iso8601TimeFormat, tm) - if err != nil { - er = ErrInvalidISO8601 - break - } + // clear ID to denote it's already been processed + data.ID = "" - fieldValue.Set(reflect.ValueOf(t)) + return nil +} - continue - } +func handleRelationUnmarshal(data *Node, args []string, fieldValue reflect.Value, included *map[string]*Node) error { + if len(args) < 2 { + return ErrBadJSONAPIStructTag + } - var at int64 + isSlice := fieldValue.Type().Kind() == reflect.Slice - if v.Kind() == reflect.Float64 { - at = int64(v.Interface().(float64)) - } else if v.Kind() == reflect.Int { - at = v.Int() - } else { - return ErrInvalidTime - } + if data.Relationships == nil || data.Relationships[args[1]] == nil { + return nil + } - t := time.Unix(at, 0) + if isSlice { + // to-many relationship + relationship := new(RelationshipManyNode) - fieldValue.Set(reflect.ValueOf(t)) + buf := bytes.NewBuffer(nil) - continue - } + json.NewEncoder(buf).Encode(data.Relationships[args[1]]) + json.NewDecoder(buf).Decode(relationship) - if fieldValue.Type() == reflect.TypeOf([]string{}) { - values := make([]string, v.Len()) - for i := 0; i < v.Len(); i++ { - values[i] = v.Index(i).Interface().(string) - } + rData := relationship.Data + models := reflect.New(fieldValue.Type()).Elem() - fieldValue.Set(reflect.ValueOf(values)) + for _, n := range rData { + m := reflect.New(fieldValue.Type().Elem().Elem()) - continue + if err := unmarshalNode( + fullNode(n, included), + m, + included, + ); err != nil { + return err } - if fieldValue.Type() == reflect.TypeOf(new(time.Time)) { - if iso8601 { - var tm string - if v.Kind() == reflect.String { - tm = v.Interface().(string) - } else { - er = ErrInvalidISO8601 - break - } - - v, err := time.Parse(iso8601TimeFormat, tm) - if err != nil { - er = ErrInvalidISO8601 - break - } + models = reflect.Append(models, m) + } - t := &v + fieldValue.Set(models) + // TODO: debug why we can't clear this + // delete(data.Relationships, args[1]) + return nil + } + // to-one relationships + relationship := new(RelationshipOneNode) + + buf := bytes.NewBuffer(nil) + + json.NewEncoder(buf).Encode( + data.Relationships[args[1]], + ) + json.NewDecoder(buf).Decode(relationship) + + /* + http://jsonapi.org/format/#document-resource-object-relationships + http://jsonapi.org/format/#document-resource-object-linkage + relationship can have a data node set to null (e.g. to disassociate the relationship) + so unmarshal and set fieldValue only if data obj is not null + */ + if relationship.Data == nil { + return nil + } - fieldValue.Set(reflect.ValueOf(t)) + m := reflect.New(fieldValue.Type().Elem()) + if err := unmarshalNode( + fullNode(relationship.Data, included), + m, + included, + ); err != nil { + return err + } - continue - } + fieldValue.Set(m) - var at int64 + // clear relation + delete(data.Relationships, args[1]) - if v.Kind() == reflect.Float64 { - at = int64(v.Interface().(float64)) - } else if v.Kind() == reflect.Int { - at = v.Int() - } else { - return ErrInvalidTime - } + return nil +} - v := time.Unix(at, 0) - t := &v +func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.StructField, fieldValue reflect.Value) error { + if len(args) < 2 { + return ErrBadJSONAPIStructTag + } + attributes := data.Attributes + if attributes == nil || len(data.Attributes) == 0 { + return nil + } - fieldValue.Set(reflect.ValueOf(t)) + var iso8601 bool - continue + if len(args) > 2 { + for _, arg := range args[2:] { + if arg == annotationISO8601 { + iso8601 = true } + } + } - // JSON value was a float (numeric) - if v.Kind() == reflect.Float64 { - floatValue := v.Interface().(float64) - - // The field may or may not be a pointer to a numeric; the kind var - // will not contain a pointer type - var kind reflect.Kind - if fieldValue.Kind() == reflect.Ptr { - kind = fieldType.Type.Elem().Kind() - } else { - kind = fieldType.Type.Kind() - } - - var numericValue reflect.Value - - switch kind { - case reflect.Int: - n := int(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Int8: - n := int8(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Int16: - n := int16(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Int32: - n := int32(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Int64: - n := int64(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Uint: - n := uint(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Uint8: - n := uint8(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Uint16: - n := uint16(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Uint32: - n := uint32(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Uint64: - n := uint64(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Float32: - n := float32(floatValue) - numericValue = reflect.ValueOf(&n) - case reflect.Float64: - n := floatValue - numericValue = reflect.ValueOf(&n) - default: - return ErrUnknownFieldNumberType - } - - assign(fieldValue, numericValue) - continue - } + val := attributes[args[1]] + + // continue if the attribute was not included in the request + if val == nil { + return nil + } + + v := reflect.ValueOf(val) - // Field was a Pointer type - if fieldValue.Kind() == reflect.Ptr { - var concreteVal reflect.Value - - switch cVal := val.(type) { - case string: - concreteVal = reflect.ValueOf(&cVal) - case bool: - concreteVal = reflect.ValueOf(&cVal) - case complex64: - concreteVal = reflect.ValueOf(&cVal) - case complex128: - concreteVal = reflect.ValueOf(&cVal) - case uintptr: - concreteVal = reflect.ValueOf(&cVal) - default: - return ErrUnsupportedPtrType - } - - if fieldValue.Type() != concreteVal.Type() { - return ErrUnsupportedPtrType - } - - fieldValue.Set(concreteVal) - continue + // Handle field of type time.Time + if fieldValue.Type() == reflect.TypeOf(time.Time{}) { + if iso8601 { + var tm string + if v.Kind() == reflect.String { + tm = v.Interface().(string) + } else { + return ErrInvalidISO8601 } - // As a final catch-all, ensure types line up to avoid a runtime panic. - // Ignore interfaces since interfaces are poly - if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != v.Kind() { - return ErrInvalidType + t, err := time.Parse(iso8601TimeFormat, tm) + if err != nil { + return ErrInvalidISO8601 } - fieldValue.Set(reflect.ValueOf(val)) - } else if annotation == annotationRelation { - isSlice := fieldValue.Type().Kind() == reflect.Slice + fieldValue.Set(reflect.ValueOf(t)) - if data.Relationships == nil || data.Relationships[args[1]] == nil { - continue - } + return nil + } - if isSlice { - // to-many relationship - relationship := new(RelationshipManyNode) + var at int64 - buf := bytes.NewBuffer(nil) + if v.Kind() == reflect.Float64 { + at = int64(v.Interface().(float64)) + } else if v.Kind() == reflect.Int { + at = v.Int() + } else { + return ErrInvalidTime + } - json.NewEncoder(buf).Encode(data.Relationships[args[1]]) - json.NewDecoder(buf).Decode(relationship) + t := time.Unix(at, 0) - data := relationship.Data - models := reflect.New(fieldValue.Type()).Elem() + fieldValue.Set(reflect.ValueOf(t)) + delete(data.Attributes, args[1]) - for _, n := range data { - m := reflect.New(fieldValue.Type().Elem().Elem()) + return nil + } - if err := unmarshalNode( - fullNode(n, included), - m, - included, - ); err != nil { - er = err - break - } + if fieldValue.Type() == reflect.TypeOf([]string{}) { + values := make([]string, v.Len()) + for i := 0; i < v.Len(); i++ { + values[i] = v.Index(i).Interface().(string) + } - models = reflect.Append(models, m) - } + fieldValue.Set(reflect.ValueOf(values)) + delete(data.Attributes, args[1]) + return nil + } - fieldValue.Set(models) + if fieldValue.Type() == reflect.TypeOf(new(time.Time)) { + if iso8601 { + var tm string + if v.Kind() == reflect.String { + tm = v.Interface().(string) } else { - // to-one relationships - relationship := new(RelationshipOneNode) - - buf := bytes.NewBuffer(nil) - - json.NewEncoder(buf).Encode( - data.Relationships[args[1]], - ) - json.NewDecoder(buf).Decode(relationship) - - /* - http://jsonapi.org/format/#document-resource-object-relationships - http://jsonapi.org/format/#document-resource-object-linkage - relationship can have a data node set to null (e.g. to disassociate the relationship) - so unmarshal and set fieldValue only if data obj is not null - */ - if relationship.Data == nil { - continue - } - - m := reflect.New(fieldValue.Type().Elem()) - if err := unmarshalNode( - fullNode(relationship.Data, included), - m, - included, - ); err != nil { - er = err - break - } - - fieldValue.Set(m) + return ErrInvalidISO8601 + + } + v, err := time.Parse(iso8601TimeFormat, tm) + if err != nil { + return ErrInvalidISO8601 } + t := &v + + fieldValue.Set(reflect.ValueOf(t)) + delete(data.Attributes, args[1]) + return nil + } + + var at int64 + + if v.Kind() == reflect.Float64 { + at = int64(v.Interface().(float64)) + } else if v.Kind() == reflect.Int { + at = v.Int() + } else { + return ErrInvalidTime + } + + v := time.Unix(at, 0) + t := &v + + fieldValue.Set(reflect.ValueOf(t)) + delete(data.Attributes, args[1]) + return nil + } + + // JSON value was a float (numeric) + if v.Kind() == reflect.Float64 { + floatValue := v.Interface().(float64) + + // The field may or may not be a pointer to a numeric; the kind var + // will not contain a pointer type + var kind reflect.Kind + if fieldValue.Kind() == reflect.Ptr { + kind = fieldType.Type.Elem().Kind() } else { - er = fmt.Errorf(unsuportedStructTagMsg, annotation) + kind = fieldType.Type.Kind() } + + var numericValue reflect.Value + + switch kind { + case reflect.Int: + n := int(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Int8: + n := int8(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Int16: + n := int16(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Int32: + n := int32(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Int64: + n := int64(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Uint: + n := uint(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Uint8: + n := uint8(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Uint16: + n := uint16(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Uint32: + n := uint32(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Uint64: + n := uint64(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Float32: + n := float32(floatValue) + numericValue = reflect.ValueOf(&n) + case reflect.Float64: + n := floatValue + numericValue = reflect.ValueOf(&n) + default: + return ErrUnknownFieldNumberType + } + + assign(fieldValue, numericValue) + delete(data.Attributes, args[1]) + return nil + } + + // Field was a Pointer type + if fieldValue.Kind() == reflect.Ptr { + var concreteVal reflect.Value + + switch cVal := val.(type) { + case string: + concreteVal = reflect.ValueOf(&cVal) + case bool: + concreteVal = reflect.ValueOf(&cVal) + case complex64: + concreteVal = reflect.ValueOf(&cVal) + case complex128: + concreteVal = reflect.ValueOf(&cVal) + case uintptr: + concreteVal = reflect.ValueOf(&cVal) + default: + return ErrUnsupportedPtrType + } + + if fieldValue.Type() != concreteVal.Type() { + return ErrUnsupportedPtrType + } + + fieldValue.Set(concreteVal) + delete(data.Attributes, args[1]) + return nil + } + + // As a final catch-all, ensure types line up to avoid a runtime panic. + // Ignore interfaces since interfaces are poly + if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != v.Kind() { + return ErrInvalidType } + fieldValue.Set(reflect.ValueOf(val)) + // clear attribute key so its not processed again + // TODO: debug why this cannot be cleared + // delete(data.Attributes, args[1]) - return er + return nil } func fullNode(n *Node, included *map[string]*Node) *Node { diff --git a/response_test.go b/response_test.go index aac265f5..f191904c 100644 --- a/response_test.go +++ b/response_test.go @@ -5,7 +5,6 @@ import ( "encoding/json" "reflect" "sort" - "strings" "testing" "time" ) @@ -985,35 +984,151 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { Bat string `jsonapi:"attr,bat"` } - model := &Model{} - model.ID = 1 - model.Fizz = "fizzy" - model.Buzz = 99 - model.Foo = "fooey" - model.Bar = "barry" - model.Bat = "batty" + type test struct { + name string + payload *OnePayload + dst, expected interface{} + } + + scenarios := []test{} + + scenarios = append(scenarios, test{ + name: "Model embeds Thing, models have no annotation overlaps", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "things", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "buzz": 99, + "fizz": "fizzy", + "foo": "fooey", + }, + }, + }, + expected: &Model{ + Foo: "fooey", + Bar: "barry", + Bat: "batty", + Thing: Thing{ + ID: 1, + Fizz: "fizzy", + Buzz: 99, + }, + }, + }) + + { + type Model struct { + Thing + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + Buzz int `jsonapi:"attr,buzz"` // overrides Thing.Buzz + } - buf := bytes.NewBuffer(nil) - if err := MarshalPayload(buf, model); err != nil { - t.Fatal(err) + scenarios = append(scenarios, test{ + name: "Model embeds Thing, overlap Buzz attribute", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "things", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "buzz": 99, + "fizz": "fizzy", + "foo": "fooey", + }, + }, + }, + expected: &Model{ + Foo: "fooey", + Bar: "barry", + Bat: "batty", + Buzz: 99, + Thing: Thing{ + ID: 1, + Fizz: "fizzy", + }, + }, + }) } - // assert encoding from model to jsonapi output - expected := `{"data":{"type":"things","id":"1","attributes":{"bar":"barry","bat":"batty","buzz":99,"fizz":"fizzy","foo":"fooey"}}}` - actual := strings.TrimSpace(string(buf.Bytes())) + { + type Model struct { + Thing + ModelID int `jsonapi:"primary,models"` //overrides Thing.ID due to primary annotation + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + Buzz int `jsonapi:"attr,buzz"` // overrides Thing.Buzz + } - if expected != actual { - t.Errorf("Got %+v Expected %+v", actual, expected) + scenarios = append(scenarios, test{ + name: "Model embeds Thing, attribute, and primary annotation overlap", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "models", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "buzz": 99, + "fizz": "fizzy", + "foo": "fooey", + }, + }, + }, + expected: &Model{ + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + Buzz: 99, + Thing: Thing{ + Fizz: "fizzy", + }, + }, + }) } + for _, scenario := range scenarios { + t.Logf("running scenario: %s\n", scenario.name) - dst := &Model{} - if err := UnmarshalPayload(buf, dst); err != nil { - t.Fatal(err) - } + // get the expected model and marshal to jsonapi + buf := bytes.NewBuffer(nil) + if err := MarshalPayload(buf, scenario.expected); err != nil { + t.Fatal(err) + } + + // get the node model representation and marshal to jsonapi + payload, err := json.Marshal(scenario.payload) + if err != nil { + t.Fatal(err) + } + + // assert that we're starting w/ the same payload + isJSONEqual, err := isJSONEqual(payload, buf.Bytes()) + if err != nil { + t.Fatal(err) + } + if !isJSONEqual { + t.Errorf("Got\n%s\nExpected\n%s\n", payload, buf.Bytes()) + } - // assert decoding from jsonapi output to model - if !reflect.DeepEqual(model, dst) { - t.Errorf("Got %#v Expected %#v", dst, model) + // run jsonapi unmarshal + if err := UnmarshalPayload(bytes.NewReader(payload), scenario.dst); err != nil { + t.Fatal(err) + } + + // assert decoded and expected models are equal + if !reflect.DeepEqual(scenario.expected, scenario.dst) { + t.Errorf("Got\n%#v\nExpected\n%#v\n", scenario.dst, scenario.expected) + } } } @@ -1083,3 +1198,17 @@ func testBlog() *Blog { }, } } + +func isJSONEqual(b1, b2 []byte) (bool, error) { + var i1, i2 interface{} + var result bool + var err error + if err = json.Unmarshal(b1, &i1); err != nil { + return result, err + } + if err = json.Unmarshal(b2, &i2); err != nil { + return result, err + } + result = reflect.DeepEqual(i1, i2) + return result, err +} From ab94c5ad9277a990e9b6cf4bf6380ab42772bbd8 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Wed, 19 Jul 2017 10:24:22 -0700 Subject: [PATCH 09/15] deep copy the node when passing relation/sideloaded notes to unmarshal() --- node.go | 32 ++++++++++++++++++++++++++++++++ request.go | 18 +++++------------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/node.go b/node.go index 6152ba4d..73b7d591 100644 --- a/node.go +++ b/node.go @@ -151,3 +151,35 @@ type RelationshipMetable interface { // JSONRelationshipMeta will be invoked for each relationship with the corresponding relation name (e.g. `comments`) JSONAPIRelationshipMeta(relation string) *Meta } + +// derefs the arg, and clones the map-type attributes +// note: maps are reference types, so they need an explicit copy. +func deepCopyNode(n *Node) *Node { + if n == nil { + return n + } + + copyMap := func(m map[string]interface{}) map[string]interface{} { + if m == nil { + return m + } + cp := make(map[string]interface{}) + for k, v := range m { + cp[k] = v + } + return cp + } + + copy := *n + copy.Attributes = copyMap(copy.Attributes) + copy.Relationships = copyMap(copy.Relationships) + if copy.Links != nil { + tmp := Links(copyMap(map[string]interface{}(*copy.Links))) + copy.Links = &tmp + } + if copy.Meta != nil { + tmp := Meta(copyMap(map[string]interface{}(*copy.Meta))) + copy.Meta = &tmp + } + return © +} diff --git a/request.go b/request.go index 29151385..706f4a70 100644 --- a/request.go +++ b/request.go @@ -156,27 +156,21 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) return ErrBadJSONAPIStructTag } - // annotation == args[0] + // args[0] == annotation switch args[0] { case annotationClientID: - //fmt.Printf("\nannotationClientID %v\n", args) if err := handleClientIDUnmarshal(data, args, fieldValue); err != nil { return err } case annotationPrimary: - //fmt.Printf("\nannotationPrimary %v\n", args) if err := handlePrimaryUnmarshal(data, args, fieldType, fieldValue); err != nil { return err } case annotationAttribute: - //fmt.Printf("\nannotationAttribute %v\n", args) - // fmt.Println("before", data.Attributes) if err := handleAttributeUnmarshal(data, args, fieldType, fieldValue); err != nil { return err } - // fmt.Println("after ", data.Attributes) case annotationRelation: - //fmt.Printf("\nannotationRelation %v\n", args) if err := handleRelationUnmarshal(data, args, fieldValue, included); err != nil { return err } @@ -340,8 +334,7 @@ func handleRelationUnmarshal(data *Node, args []string, fieldValue reflect.Value } fieldValue.Set(models) - // TODO: debug why we can't clear this - // delete(data.Relationships, args[1]) + delete(data.Relationships, args[1]) return nil } // to-one relationships @@ -594,8 +587,7 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc } fieldValue.Set(reflect.ValueOf(val)) // clear attribute key so its not processed again - // TODO: debug why this cannot be cleared - // delete(data.Attributes, args[1]) + delete(data.Attributes, args[1]) return nil } @@ -604,10 +596,10 @@ func fullNode(n *Node, included *map[string]*Node) *Node { includedKey := fmt.Sprintf("%s,%s", n.Type, n.ID) if included != nil && (*included)[includedKey] != nil { - return (*included)[includedKey] + return deepCopyNode((*included)[includedKey]) } - return n + return deepCopyNode(n) } // assign will take the value specified and assign it to the field; if From 7d26540f503e54027b31ade83835c0c9e8b173cf Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Wed, 19 Jul 2017 11:34:35 -0700 Subject: [PATCH 10/15] add some comments and do some additional cleanup --- request.go | 211 ++++++++++++++++++++++++++++------------------------- 1 file changed, 110 insertions(+), 101 deletions(-) diff --git a/request.go b/request.go index 706f4a70..fa7786f4 100644 --- a/request.go +++ b/request.go @@ -117,7 +117,11 @@ func UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) { return models, nil } -// TODO: refactor so that it unmarshals from top to bottom +// unmarshalNode handles embedded struct models from top to down. +// it loops through the struct fields, handles attributes/relations at that level first +// the handling the embedded structs are done last, so that you get the expected composition behavior +// data (*Node) attributes are cleared on each success. +// relations/sideloaded models use deeply copied Nodes (since those sideloaded models can be referenced in multiple relations) func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) (err error) { defer func() { if r := recover(); r != nil { @@ -197,9 +201,8 @@ func handleClientIDUnmarshal(data *Node, args []string, fieldValue reflect.Value return nil } + // set value and clear clientID to denote it's already been processed fieldValue.Set(reflect.ValueOf(data.ClientID)) - - // clear clientID to denote it's already been processed data.ClientID = "" return nil @@ -223,9 +226,6 @@ func handlePrimaryUnmarshal(data *Node, args []string, fieldType reflect.StructF ) } - // ID will have to be transmitted as astring per the JSON API spec - v := reflect.ValueOf(data.ID) - // Deal with PTRS var kind reflect.Kind if fieldValue.Kind() == reflect.Ptr { @@ -234,63 +234,63 @@ func handlePrimaryUnmarshal(data *Node, args []string, fieldType reflect.StructF kind = fieldType.Type.Kind() } + var idValue reflect.Value + // Handle String case if kind == reflect.String { - assign(fieldValue, v) - return nil - } - - // Value was not a string... only other supported type was a numeric, - // which would have been sent as a float value. - floatValue, err := strconv.ParseFloat(data.ID, 64) - if err != nil { - // Could not convert the value in the "id" attr to a float - return ErrBadJSONAPIID - } + // ID will have to be transmitted as a string per the JSON API spec + idValue = reflect.ValueOf(data.ID) + } else { + // Value was not a string... only other supported type was a numeric, + // which would have been sent as a float value. + floatValue, err := strconv.ParseFloat(data.ID, 64) + if err != nil { + // Could not convert the value in the "id" attr to a float + return ErrBadJSONAPIID + } - // Convert the numeric float to one of the supported ID numeric types - // (int[8,16,32,64] or uint[8,16,32,64]) - var idValue reflect.Value - switch kind { - case reflect.Int: - n := int(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int8: - n := int8(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int16: - n := int16(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int32: - n := int32(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Int64: - n := int64(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint: - n := uint(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint8: - n := uint8(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint16: - n := uint16(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint32: - n := uint32(floatValue) - idValue = reflect.ValueOf(&n) - case reflect.Uint64: - n := uint64(floatValue) - idValue = reflect.ValueOf(&n) - default: - // We had a JSON float (numeric), but our field was not one of the - // allowed numeric types - return ErrBadJSONAPIID + // Convert the numeric float to one of the supported ID numeric types + // (int[8,16,32,64] or uint[8,16,32,64]) + switch kind { + case reflect.Int: + n := int(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int8: + n := int8(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int16: + n := int16(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int32: + n := int32(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Int64: + n := int64(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint: + n := uint(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint8: + n := uint8(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint16: + n := uint16(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint32: + n := uint32(floatValue) + idValue = reflect.ValueOf(&n) + case reflect.Uint64: + n := uint64(floatValue) + idValue = reflect.ValueOf(&n) + default: + // We had a JSON float (numeric), but our field was not one of the + // allowed numeric types + return ErrBadJSONAPIID + } } + // set value and clear ID to denote it's already been processed assign(fieldValue, idValue) - - // clear ID to denote it's already been processed data.ID = "" return nil @@ -301,52 +301,39 @@ func handleRelationUnmarshal(data *Node, args []string, fieldValue reflect.Value return ErrBadJSONAPIStructTag } - isSlice := fieldValue.Type().Kind() == reflect.Slice - if data.Relationships == nil || data.Relationships[args[1]] == nil { return nil } + // to-one relationships + handler := handleToOneRelationUnmarshal + isSlice := fieldValue.Type().Kind() == reflect.Slice if isSlice { // to-many relationship - relationship := new(RelationshipManyNode) - - buf := bytes.NewBuffer(nil) - - json.NewEncoder(buf).Encode(data.Relationships[args[1]]) - json.NewDecoder(buf).Decode(relationship) - - rData := relationship.Data - models := reflect.New(fieldValue.Type()).Elem() - - for _, n := range rData { - m := reflect.New(fieldValue.Type().Elem().Elem()) - - if err := unmarshalNode( - fullNode(n, included), - m, - included, - ); err != nil { - return err - } - - models = reflect.Append(models, m) - } + handler = handleToManyRelationUnmarshal + } - fieldValue.Set(models) - delete(data.Relationships, args[1]) - return nil + v, err := handler(data.Relationships[args[1]], fieldValue.Type(), included) + if err != nil { + return err } - // to-one relationships + // set only if there is a val since val can be null (e.g. to disassociate the relationship) + if v != nil { + fieldValue.Set(*v) + } + delete(data.Relationships, args[1]) + return nil +} + +// to-one relationships +func handleToOneRelationUnmarshal(relationData interface{}, fieldType reflect.Type, included *map[string]*Node) (*reflect.Value, error) { relationship := new(RelationshipOneNode) buf := bytes.NewBuffer(nil) - - json.NewEncoder(buf).Encode( - data.Relationships[args[1]], - ) + json.NewEncoder(buf).Encode(relationData) json.NewDecoder(buf).Decode(relationship) + m := reflect.New(fieldType.Elem()) /* http://jsonapi.org/format/#document-resource-object-relationships http://jsonapi.org/format/#document-resource-object-linkage @@ -354,26 +341,49 @@ func handleRelationUnmarshal(data *Node, args []string, fieldValue reflect.Value so unmarshal and set fieldValue only if data obj is not null */ if relationship.Data == nil { - return nil + return nil, nil } - m := reflect.New(fieldValue.Type().Elem()) if err := unmarshalNode( fullNode(relationship.Data, included), m, included, ); err != nil { - return err + return nil, err } - fieldValue.Set(m) + return &m, nil +} - // clear relation - delete(data.Relationships, args[1]) +// to-many relationship +func handleToManyRelationUnmarshal(relationData interface{}, fieldType reflect.Type, included *map[string]*Node) (*reflect.Value, error) { + relationship := new(RelationshipManyNode) - return nil + buf := bytes.NewBuffer(nil) + json.NewEncoder(buf).Encode(relationData) + json.NewDecoder(buf).Decode(relationship) + + models := reflect.New(fieldType).Elem() + + rData := relationship.Data + for _, n := range rData { + m := reflect.New(fieldType.Elem().Elem()) + + if err := unmarshalNode( + fullNode(n, included), + m, + included, + ); err != nil { + return nil, err + } + + models = reflect.Append(models, m) + } + + return &models, nil } +// TODO: break this out into smaller funcs func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.StructField, fieldValue reflect.Value) error { if len(args) < 2 { return ErrBadJSONAPIStructTag @@ -418,7 +428,7 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc } fieldValue.Set(reflect.ValueOf(t)) - + delete(data.Attributes, args[1]) return nil } @@ -436,7 +446,6 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc fieldValue.Set(reflect.ValueOf(t)) delete(data.Attributes, args[1]) - return nil } @@ -585,10 +594,10 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != v.Kind() { return ErrInvalidType } + + // set val and clear attribute key so its not processed again fieldValue.Set(reflect.ValueOf(val)) - // clear attribute key so its not processed again delete(data.Attributes, args[1]) - return nil } From f79a192d0c7011204032350b84fbafb91bddb957 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Wed, 19 Jul 2017 12:49:13 -0700 Subject: [PATCH 11/15] add test uses annotationIgnore --- response_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/response_test.go b/response_test.go index f191904c..62c16db9 100644 --- a/response_test.go +++ b/response_test.go @@ -1096,6 +1096,41 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { }, }) } + + { + type Model struct { + Thing `jsonapi:"-"` + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + Buzz int `jsonapi:"attr,buzz"` + } + + scenarios = append(scenarios, test{ + name: "Model embeds Thing, but is annotated w/ ignore", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "models", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "buzz": 99, + "foo": "fooey", + }, + }, + }, + expected: &Model{ + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + Buzz: 99, + }, + }) + } for _, scenario := range scenarios { t.Logf("running scenario: %s\n", scenario.name) From deeffb78df43b641cb10e125bfdb021deb14bc72 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Thu, 20 Jul 2017 13:42:57 -0700 Subject: [PATCH 12/15] implement support for struct fields that implement json.Marshaler/Unmarshaler --- attributes.go | 69 +++++++++++++++++++++++++++++++++ attributes_test.go | 95 ++++++++++++++++++++++++++++++++++++++++++++++ request.go | 34 +++++++++++++++++ response.go | 12 ++++-- response_test.go | 37 ++++++++++-------- 5 files changed, 229 insertions(+), 18 deletions(-) create mode 100644 attributes.go create mode 100644 attributes_test.go diff --git a/attributes.go b/attributes.go new file mode 100644 index 00000000..8e0f4c3a --- /dev/null +++ b/attributes.go @@ -0,0 +1,69 @@ +package jsonapi + +import ( + "encoding/json" + "reflect" + "strconv" + "time" +) + +const iso8601Layout = "2006-01-02T15:04:05Z07:00" + +// ISO8601Datetime represents a ISO8601 formatted datetime +// It is a time.Time instance that marshals and unmarshals to the ISO8601 ref +type ISO8601Datetime struct { + time.Time +} + +// MarshalJSON implements the json.Marshaler interface. +func (t *ISO8601Datetime) MarshalJSON() ([]byte, error) { + s := t.Time.Format(iso8601Layout) + return json.Marshal(s) +} + +// UnmarshalJSON implements the json.Unmarshaler interface. +func (t *ISO8601Datetime) UnmarshalJSON(data []byte) error { + // Ignore null, like in the main JSON package. + if string(data) == "null" { + return nil + } + // Fractional seconds are handled implicitly by Parse. + var err error + t.Time, err = time.Parse(strconv.Quote(iso8601Layout), string(data)) + return err +} + +// ISO8601Datetime.String() - override default String() on time +func (t ISO8601Datetime) String() string { + return t.Format(iso8601Layout) +} + +// func to help determine json.Marshaler implementation +// checks both pointer and non-pointer implementations +func isJSONMarshaler(fv reflect.Value) (json.Marshaler, bool) { + if u, ok := fv.Interface().(json.Marshaler); ok { + return u, ok + } + + if !fv.CanAddr() { + return nil, false + } + + u, ok := fv.Addr().Interface().(json.Marshaler) + return u, ok +} + +// func to help determine json.Unmarshaler implementation +// checks both pointer and non-pointer implementations +func isJSONUnmarshaler(fv reflect.Value) (json.Unmarshaler, bool) { + if u, ok := fv.Interface().(json.Unmarshaler); ok { + return u, ok + } + + if !fv.CanAddr() { + return nil, false + } + + u, ok := fv.Addr().Interface().(json.Unmarshaler) + return u, ok +} diff --git a/attributes_test.go b/attributes_test.go new file mode 100644 index 00000000..b60b68b2 --- /dev/null +++ b/attributes_test.go @@ -0,0 +1,95 @@ +package jsonapi + +import ( + "reflect" + "strconv" + "testing" + "time" +) + +func TestISO8601Datetime(t *testing.T) { + pacific, err := time.LoadLocation("America/Los_Angeles") + if err != nil { + t.Fatal(err) + } + + type test struct { + stringVal string + dtVal ISO8601Datetime + } + + tests := []*test{ + &test{ + stringVal: strconv.Quote("2017-04-06T13:00:00-07:00"), + dtVal: ISO8601Datetime{Time: time.Date(2017, time.April, 6, 13, 0, 0, 0, pacific)}, + }, + &test{ + stringVal: strconv.Quote("2007-05-06T13:00:00-07:00"), + dtVal: ISO8601Datetime{Time: time.Date(2007, time.May, 6, 13, 0, 0, 0, pacific)}, + }, + &test{ + stringVal: strconv.Quote("2016-12-08T15:18:54Z"), + dtVal: ISO8601Datetime{Time: time.Date(2016, time.December, 8, 15, 18, 54, 0, time.UTC)}, + }, + } + + for _, test := range tests { + // unmarshal stringVal by calling UnmarshalJSON() + dt := &ISO8601Datetime{} + if err := dt.UnmarshalJSON([]byte(test.stringVal)); err != nil { + t.Fatal(err) + } + + // compare unmarshaled stringVal to dtVal + if !dt.Time.Equal(test.dtVal.Time) { + t.Errorf("\n\tE=%+v\n\tA=%+v", test.dtVal.UnixNano(), dt.UnixNano()) + } + + // marshal dtVal by calling MarshalJSON() + b, err := test.dtVal.MarshalJSON() + if err != nil { + t.Fatal(err) + } + + // compare marshaled dtVal to stringVal + if test.stringVal != string(b) { + t.Errorf("\n\tE=%+v\n\tA=%+v", test.stringVal, string(b)) + } + } +} + +func TestIsJSONMarshaler(t *testing.T) { + { // positive + isoDateTime := ISO8601Datetime{} + v := reflect.ValueOf(&isoDateTime) + if _, ok := isJSONMarshaler(v); !ok { + t.Error("got false; expected ISO8601Datetime to implement json.Marshaler") + } + } + { // negative + type customString string + input := customString("foo") + v := reflect.ValueOf(&input) + if _, ok := isJSONMarshaler(v); ok { + t.Error("got true; expected customString to not implement json.Marshaler") + } + } +} + +func TestIsJSONUnmarshaler(t *testing.T) { + { // positive + isoDateTime := ISO8601Datetime{} + v := reflect.ValueOf(&isoDateTime) + if _, ok := isJSONUnmarshaler(v); !ok { + t.Error("expected ISO8601Datetime to implement json.Unmarshaler") + } + } + { // negative + type customString string + input := customString("foo") + v := reflect.ValueOf(&input) + if _, ok := isJSONUnmarshaler(v); ok { + t.Error("got true; expected customString to not implement json.Unmarshaler") + } + } +} diff --git a/request.go b/request.go index fa7786f4..93ad1d95 100644 --- a/request.go +++ b/request.go @@ -589,6 +589,11 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc return nil } + // TODO: refactor the time type handlers to implement json.Unmarshaler and move this higher + if isJSONUnmarshaler, err := handleJSONUnmarshalerAttributes(data, args, fieldValue); isJSONUnmarshaler { + return err + } + // As a final catch-all, ensure types line up to avoid a runtime panic. // Ignore interfaces since interfaces are poly if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != v.Kind() { @@ -601,6 +606,35 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc return nil } +func handleJSONUnmarshalerAttributes(data *Node, args []string, fieldValue reflect.Value) (bool, error) { + val := data.Attributes[args[1]] + + // handle struct fields that implement json.Unmarshaler + if fieldValue.Type().NumMethod() > 0 { + jsonUnmarshaler, ok := isJSONUnmarshaler(fieldValue) + // if field doesn't implement the json.Unmarshaler, ignore and move on + if !ok { + return ok, nil + } + + b, err := json.Marshal(val) + if err != nil { + return ok, err + } + + if err := jsonUnmarshaler.UnmarshalJSON(b); err != nil { + return ok, err + } + + // success; clear value + delete(data.Attributes, args[1]) + return ok, nil + } + + // field does not implement any methods, including json.Unmarshaler; continue + return false, nil +} + func fullNode(n *Node, included *map[string]*Node) *Node { includedKey := fmt.Sprintf("%s,%s", n.Type, n.ID) diff --git a/response.go b/response.go index 0f34df2b..8ce1f2a7 100644 --- a/response.go +++ b/response.go @@ -215,11 +215,12 @@ func visitModelNode(model interface{}, included *map[string]*Node, structField := modelValue.Type().Field(i) tag := structField.Tag.Get(annotationJSONAPI) + if shouldIgnoreField(tag) { + continue + } + // handles embedded structs if isEmbeddedStruct(structField) { - if shouldIgnoreField(tag) { - continue - } model := modelValue.Field(i).Addr().Interface() embNode, err := visitModelNode(model, included, sideload) if err != nil { @@ -360,6 +361,11 @@ func visitModelNode(model interface{}, included *map[string]*Node, continue } + if jsonMarshaler, ok := isJSONMarshaler(fieldValue); ok { + node.Attributes[args[1]] = jsonMarshaler + continue + } + strAttr, ok := fieldValue.Interface().(string) if ok { node.Attributes[args[1]] = strAttr diff --git a/response_test.go b/response_test.go index 62c16db9..7380b193 100644 --- a/response_test.go +++ b/response_test.go @@ -1099,12 +1099,17 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { { type Model struct { - Thing `jsonapi:"-"` - ModelID int `jsonapi:"primary,models"` - Foo string `jsonapi:"attr,foo"` - Bar string `jsonapi:"attr,bar"` - Bat string `jsonapi:"attr,bat"` - Buzz int `jsonapi:"attr,buzz"` + Thing `jsonapi:"-"` + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + Buzz int `jsonapi:"attr,buzz"` + CreateDate ISO8601Datetime `jsonapi:"attr,create-date"` + } + + isoDate := ISO8601Datetime{ + Time: time.Date(2016, time.December, 8, 15, 18, 54, 0, time.UTC), } scenarios = append(scenarios, test{ @@ -1115,19 +1120,21 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { Type: "models", ID: "1", Attributes: map[string]interface{}{ - "bar": "barry", - "bat": "batty", - "buzz": 99, - "foo": "fooey", + "bar": "barry", + "bat": "batty", + "buzz": 99, + "foo": "fooey", + "create-date": isoDate.String(), }, }, }, expected: &Model{ - ModelID: 1, - Foo: "fooey", - Bar: "barry", - Bat: "batty", - Buzz: 99, + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + Buzz: 99, + CreateDate: isoDate, }, }) } From c66d1da8ca2fb9f3d74b49582de5235699d5004a Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Thu, 20 Jul 2017 15:59:52 -0700 Subject: [PATCH 13/15] add additional test that compares marshal/unmarshal behavior w/ standard json library --- response_test.go | 137 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/response_test.go b/response_test.go index 7380b193..461c70ab 100644 --- a/response_test.go +++ b/response_test.go @@ -970,6 +970,143 @@ func TestIsValidEmbeddedStruct(t *testing.T) { } } +// TestEmbeddedUnmarshalOrder tests the behavior of the marshaler/unmarshaler of embedded structs +// when a struct has an embedded struct w/ competing attributes, the top-level attributes take precedence +// it compares the behavior against the standard json package +func TestEmbeddedUnmarshalOrder(t *testing.T) { + type Bar struct { + Name int `jsonapi:"attr,Name"` + } + + type Foo struct { + Bar + ID string `jsonapi:"primary,foos"` + Name string `jsonapi:"attr,Name"` + } + + f := &Foo{ + ID: "1", + Name: "foo", + Bar: Bar{ + Name: 5, + }, + } + + // marshal f (Foo) using jsonapi marshaler + jsonAPIData := bytes.NewBuffer(nil) + if err := MarshalPayload(jsonAPIData, f); err != nil { + t.Fatal(err) + } + + // marshal f (Foo) using json marshaler + jsonData, err := json.Marshal(f) + + // convert bytes to map[string]interface{} so that we can do a semantic JSON comparison + var jsonAPIVal, jsonVal map[string]interface{} + if err := json.Unmarshal(jsonAPIData.Bytes(), &jsonAPIVal); err != nil { + t.Fatal(err) + } + if err = json.Unmarshal(jsonData, &jsonVal); err != nil { + t.Fatal(err) + } + + // get to the jsonapi attribute map + jAttrMap := jsonAPIVal["data"].(map[string]interface{})["attributes"].(map[string]interface{}) + + // compare + if !reflect.DeepEqual(jAttrMap["Name"], jsonVal["Name"]) { + t.Errorf("Got\n%s\nExpected\n%s\n", jAttrMap["Name"], jsonVal["Name"]) + } +} + +// TestEmbeddedMarshalOrder tests the behavior of the marshaler/unmarshaler of embedded structs +// when a struct has an embedded struct w/ competing attributes, the top-level attributes take precedence +// it compares the behavior against the standard json package +func TestEmbeddedMarshalOrder(t *testing.T) { + type Bar struct { + Name int `jsonapi:"attr,Name"` + } + + type Foo struct { + Bar + ID string `jsonapi:"primary,foos"` + Name string `jsonapi:"attr,Name"` + } + + // get a jsonapi payload w/ Name attribute of an int type + payloadWithInt, err := json.Marshal(&OnePayload{ + Data: &Node{ + Type: "foos", + ID: "1", + Attributes: map[string]interface{}{ + "Name": 5, + }, + }, + }) + if err != nil { + t.Fatal(err) + } + + // get a jsonapi payload w/ Name attribute of an string type + payloadWithString, err := json.Marshal(&OnePayload{ + Data: &Node{ + Type: "foos", + ID: "1", + Attributes: map[string]interface{}{ + "Name": "foo", + }, + }, + }) + if err != nil { + t.Fatal(err) + } + + // unmarshal payloadWithInt to f (Foo) using jsonapi unmarshaler; expecting an error + f := &Foo{} + if err := UnmarshalPayload(bytes.NewReader(payloadWithInt), f); err == nil { + t.Errorf("expected an error: int value of 5 should attempt to map to Foo.Name (string) and error") + } + + // unmarshal payloadWithString to f (Foo) using jsonapi unmarshaler; expecting no error + f = &Foo{} + if err := UnmarshalPayload(bytes.NewReader(payloadWithString), f); err != nil { + t.Error(err) + } + if f.Name != "foo" { + t.Errorf("Got\n%s\nExpected\n%s\n", "foo", f.Name) + } + + // get a json payload w/ Name attribute of an int type + bWithInt, err := json.Marshal(map[string]interface{}{ + "Name": 5, + }) + if err != nil { + t.Fatal(err) + } + + // get a json payload w/ Name attribute of an string type + bWithString, err := json.Marshal(map[string]interface{}{ + "Name": "foo", + }) + if err != nil { + t.Fatal(err) + } + + // unmarshal bWithInt to f (Foo) using json unmarshaler; expecting an error + f = &Foo{} + if err := json.Unmarshal(bWithInt, f); err == nil { + t.Errorf("expected an error: int value of 5 should attempt to map to Foo.Name (string) and error") + } + // unmarshal bWithString to f (Foo) using json unmarshaler; expecting no error + f = &Foo{} + if err := json.Unmarshal(bWithString, f); err != nil { + t.Error(err) + } + if f.Name != "foo" { + t.Errorf("Got\n%s\nExpected\n%s\n", "foo", f.Name) + } +} + func TestMarshalUnmarshalCompositeStruct(t *testing.T) { type Thing struct { ID int `jsonapi:"primary,things"` From 218abd9b66bea6a0bd5fd3e0f9f67a031b5795a8 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 21 Jul 2017 12:30:53 -0700 Subject: [PATCH 14/15] add support for pointer embedded structs --- request.go | 45 +++++++++++++-- response.go | 31 ++++++++--- response_test.go | 139 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 200 insertions(+), 15 deletions(-) diff --git a/request.go b/request.go index 93ad1d95..fda14148 100644 --- a/request.go +++ b/request.go @@ -132,7 +132,10 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) modelValue := model.Elem() modelType := model.Type().Elem() - embeddedModels := []reflect.Value{} + type embedded struct { + structField, model reflect.Value + } + embeddeds := []*embedded{} for i := 0; i < modelValue.NumField(); i++ { fieldType := modelType.Field(i) @@ -146,7 +149,24 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) // handles embedded structs if isEmbeddedStruct(fieldType) { - embeddedModels = append(embeddedModels, reflect.ValueOf(fieldValue.Addr().Interface())) + embeddeds = append(embeddeds, + &embedded{ + model: reflect.ValueOf(fieldValue.Addr().Interface()), + structField: fieldValue, + }, + ) + continue + } + + // handles pointers to embedded structs + if isEmbeddedStructPtr(fieldType) { + embeddeds = append(embeddeds, + &embedded{ + model: reflect.ValueOf(fieldValue.Interface()), + structField: fieldValue, + }, + ) + continue } // handle tagless; after handling embedded structs (which could be tagless) @@ -182,11 +202,26 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) return fmt.Errorf(unsuportedStructTagMsg, args[0]) } } + // handle embedded last - for _, em := range embeddedModels { - if err := unmarshalNode(data, em, included); err != nil { - return err + for _, em := range embeddeds { + // if nil, need to construct and rollback accordingly + if em.model.IsNil() { + copy := deepCopyNode(data) + tmp := reflect.New(em.model.Type().Elem()) + if err := unmarshalNode(copy, tmp, included); err != nil { + return err + } + + // had changes; assign value to struct field, replace orig node (data) w/ mutated copy + if !reflect.DeepEqual(copy, data) { + assign(em.structField, tmp) + data = copy + } + return nil } + // handle non-nil scenarios + return unmarshalNode(data, em.model, included) } return nil diff --git a/response.go b/response.go index 8ce1f2a7..e73e4521 100644 --- a/response.go +++ b/response.go @@ -212,31 +212,40 @@ func visitModelNode(model interface{}, included *map[string]*Node, modelType := reflect.ValueOf(model).Type().Elem() for i := 0; i < modelValue.NumField(); i++ { - structField := modelValue.Type().Field(i) - tag := structField.Tag.Get(annotationJSONAPI) + fieldValue := modelValue.Field(i) + fieldType := modelType.Field(i) + + tag := fieldType.Tag.Get(annotationJSONAPI) if shouldIgnoreField(tag) { continue } - // handles embedded structs - if isEmbeddedStruct(structField) { - model := modelValue.Field(i).Addr().Interface() - embNode, err := visitModelNode(model, included, sideload) + // handles embedded structs and pointers to embedded structs + if isEmbeddedStruct(fieldType) || isEmbeddedStructPtr(fieldType) { + var embModel interface{} + if fieldType.Type.Kind() == reflect.Ptr { + if fieldValue.IsNil() { + continue + } + embModel = fieldValue.Interface() + } else { + embModel = fieldValue.Addr().Interface() + } + + embNode, err := visitModelNode(embModel, included, sideload) if err != nil { er = err break } node.merge(embNode) + continue } if tag == "" { continue } - fieldValue := modelValue.Field(i) - fieldType := modelType.Field(i) - args := strings.Split(tag, annotationSeperator) if len(args) < 1 { @@ -559,6 +568,10 @@ func isEmbeddedStruct(sField reflect.StructField) bool { return sField.Anonymous && sField.Type.Kind() == reflect.Struct } +func isEmbeddedStructPtr(sField reflect.StructField) bool { + return sField.Anonymous && sField.Type.Kind() == reflect.Ptr && sField.Type.Elem().Kind() == reflect.Struct +} + func shouldIgnoreField(japiTag string) bool { return strings.HasPrefix(japiTag, annotationIgnore) } diff --git a/response_test.go b/response_test.go index 461c70ab..d38977c1 100644 --- a/response_test.go +++ b/response_test.go @@ -1275,6 +1275,143 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { }, }) } + { + type Model struct { + *Thing + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + } + + scenarios = append(scenarios, test{ + name: "Model embeds pointer of Thing; Thing is initialized in advance", + dst: &Model{Thing: &Thing{}}, + payload: &OnePayload{ + Data: &Node{ + Type: "models", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "foo": "fooey", + "buzz": 99, + "fizz": "fizzy", + }, + }, + }, + expected: &Model{ + Thing: &Thing{ + Fizz: "fizzy", + Buzz: 99, + }, + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + }, + }) + } + { + type Model struct { + *Thing + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + } + + scenarios = append(scenarios, test{ + name: "Model embeds pointer of Thing; Thing is initialized w/ Unmarshal", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "models", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "foo": "fooey", + "buzz": 99, + "fizz": "fizzy", + }, + }, + }, + expected: &Model{ + Thing: &Thing{ + Fizz: "fizzy", + Buzz: 99, + }, + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + }, + }) + } + { + type Model struct { + *Thing + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + } + + scenarios = append(scenarios, test{ + name: "Model embeds pointer of Thing; jsonapi model doesn't assign anything to Thing; *Thing is nil", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "models", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "foo": "fooey", + }, + }, + }, + expected: &Model{ + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + }, + }) + } + + { + type Model struct { + *Thing + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + } + + scenarios = append(scenarios, test{ + name: "Model embeds pointer of Thing; *Thing is nil", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "models", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "foo": "fooey", + }, + }, + }, + expected: &Model{ + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + }, + }) + } for _, scenario := range scenarios { t.Logf("running scenario: %s\n", scenario.name) @@ -1296,7 +1433,7 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { t.Fatal(err) } if !isJSONEqual { - t.Errorf("Got\n%s\nExpected\n%s\n", payload, buf.Bytes()) + t.Errorf("Got\n%s\nExpected\n%s\n", buf.Bytes(), payload) } // run jsonapi unmarshal From f491beefc628647110abde1d4b033250741e55da Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Wed, 26 Jul 2017 09:05:40 -0700 Subject: [PATCH 15/15] Revert "implement support for struct fields that implement json.Marshaler/Unmarshaler" This reverts commit deeffb78df43b641cb10e125bfdb021deb14bc72. --- attributes.go | 69 --------------------------------- attributes_test.go | 95 ---------------------------------------------- request.go | 34 ----------------- response.go | 5 --- response_test.go | 37 ++++++++---------- 5 files changed, 15 insertions(+), 225 deletions(-) delete mode 100644 attributes.go delete mode 100644 attributes_test.go diff --git a/attributes.go b/attributes.go deleted file mode 100644 index 8e0f4c3a..00000000 --- a/attributes.go +++ /dev/null @@ -1,69 +0,0 @@ -package jsonapi - -import ( - "encoding/json" - "reflect" - "strconv" - "time" -) - -const iso8601Layout = "2006-01-02T15:04:05Z07:00" - -// ISO8601Datetime represents a ISO8601 formatted datetime -// It is a time.Time instance that marshals and unmarshals to the ISO8601 ref -type ISO8601Datetime struct { - time.Time -} - -// MarshalJSON implements the json.Marshaler interface. -func (t *ISO8601Datetime) MarshalJSON() ([]byte, error) { - s := t.Time.Format(iso8601Layout) - return json.Marshal(s) -} - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (t *ISO8601Datetime) UnmarshalJSON(data []byte) error { - // Ignore null, like in the main JSON package. - if string(data) == "null" { - return nil - } - // Fractional seconds are handled implicitly by Parse. - var err error - t.Time, err = time.Parse(strconv.Quote(iso8601Layout), string(data)) - return err -} - -// ISO8601Datetime.String() - override default String() on time -func (t ISO8601Datetime) String() string { - return t.Format(iso8601Layout) -} - -// func to help determine json.Marshaler implementation -// checks both pointer and non-pointer implementations -func isJSONMarshaler(fv reflect.Value) (json.Marshaler, bool) { - if u, ok := fv.Interface().(json.Marshaler); ok { - return u, ok - } - - if !fv.CanAddr() { - return nil, false - } - - u, ok := fv.Addr().Interface().(json.Marshaler) - return u, ok -} - -// func to help determine json.Unmarshaler implementation -// checks both pointer and non-pointer implementations -func isJSONUnmarshaler(fv reflect.Value) (json.Unmarshaler, bool) { - if u, ok := fv.Interface().(json.Unmarshaler); ok { - return u, ok - } - - if !fv.CanAddr() { - return nil, false - } - - u, ok := fv.Addr().Interface().(json.Unmarshaler) - return u, ok -} diff --git a/attributes_test.go b/attributes_test.go deleted file mode 100644 index b60b68b2..00000000 --- a/attributes_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package jsonapi - -import ( - "reflect" - "strconv" - "testing" - "time" -) - -func TestISO8601Datetime(t *testing.T) { - pacific, err := time.LoadLocation("America/Los_Angeles") - if err != nil { - t.Fatal(err) - } - - type test struct { - stringVal string - dtVal ISO8601Datetime - } - - tests := []*test{ - &test{ - stringVal: strconv.Quote("2017-04-06T13:00:00-07:00"), - dtVal: ISO8601Datetime{Time: time.Date(2017, time.April, 6, 13, 0, 0, 0, pacific)}, - }, - &test{ - stringVal: strconv.Quote("2007-05-06T13:00:00-07:00"), - dtVal: ISO8601Datetime{Time: time.Date(2007, time.May, 6, 13, 0, 0, 0, pacific)}, - }, - &test{ - stringVal: strconv.Quote("2016-12-08T15:18:54Z"), - dtVal: ISO8601Datetime{Time: time.Date(2016, time.December, 8, 15, 18, 54, 0, time.UTC)}, - }, - } - - for _, test := range tests { - // unmarshal stringVal by calling UnmarshalJSON() - dt := &ISO8601Datetime{} - if err := dt.UnmarshalJSON([]byte(test.stringVal)); err != nil { - t.Fatal(err) - } - - // compare unmarshaled stringVal to dtVal - if !dt.Time.Equal(test.dtVal.Time) { - t.Errorf("\n\tE=%+v\n\tA=%+v", test.dtVal.UnixNano(), dt.UnixNano()) - } - - // marshal dtVal by calling MarshalJSON() - b, err := test.dtVal.MarshalJSON() - if err != nil { - t.Fatal(err) - } - - // compare marshaled dtVal to stringVal - if test.stringVal != string(b) { - t.Errorf("\n\tE=%+v\n\tA=%+v", test.stringVal, string(b)) - } - } -} - -func TestIsJSONMarshaler(t *testing.T) { - { // positive - isoDateTime := ISO8601Datetime{} - v := reflect.ValueOf(&isoDateTime) - if _, ok := isJSONMarshaler(v); !ok { - t.Error("got false; expected ISO8601Datetime to implement json.Marshaler") - } - } - { // negative - type customString string - input := customString("foo") - v := reflect.ValueOf(&input) - if _, ok := isJSONMarshaler(v); ok { - t.Error("got true; expected customString to not implement json.Marshaler") - } - } -} - -func TestIsJSONUnmarshaler(t *testing.T) { - { // positive - isoDateTime := ISO8601Datetime{} - v := reflect.ValueOf(&isoDateTime) - if _, ok := isJSONUnmarshaler(v); !ok { - t.Error("expected ISO8601Datetime to implement json.Unmarshaler") - } - } - { // negative - type customString string - input := customString("foo") - v := reflect.ValueOf(&input) - if _, ok := isJSONUnmarshaler(v); ok { - t.Error("got true; expected customString to not implement json.Unmarshaler") - } - } -} diff --git a/request.go b/request.go index fda14148..9e0eb1a3 100644 --- a/request.go +++ b/request.go @@ -624,11 +624,6 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc return nil } - // TODO: refactor the time type handlers to implement json.Unmarshaler and move this higher - if isJSONUnmarshaler, err := handleJSONUnmarshalerAttributes(data, args, fieldValue); isJSONUnmarshaler { - return err - } - // As a final catch-all, ensure types line up to avoid a runtime panic. // Ignore interfaces since interfaces are poly if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != v.Kind() { @@ -641,35 +636,6 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc return nil } -func handleJSONUnmarshalerAttributes(data *Node, args []string, fieldValue reflect.Value) (bool, error) { - val := data.Attributes[args[1]] - - // handle struct fields that implement json.Unmarshaler - if fieldValue.Type().NumMethod() > 0 { - jsonUnmarshaler, ok := isJSONUnmarshaler(fieldValue) - // if field doesn't implement the json.Unmarshaler, ignore and move on - if !ok { - return ok, nil - } - - b, err := json.Marshal(val) - if err != nil { - return ok, err - } - - if err := jsonUnmarshaler.UnmarshalJSON(b); err != nil { - return ok, err - } - - // success; clear value - delete(data.Attributes, args[1]) - return ok, nil - } - - // field does not implement any methods, including json.Unmarshaler; continue - return false, nil -} - func fullNode(n *Node, included *map[string]*Node) *Node { includedKey := fmt.Sprintf("%s,%s", n.Type, n.ID) diff --git a/response.go b/response.go index e73e4521..2e9acd75 100644 --- a/response.go +++ b/response.go @@ -370,11 +370,6 @@ func visitModelNode(model interface{}, included *map[string]*Node, continue } - if jsonMarshaler, ok := isJSONMarshaler(fieldValue); ok { - node.Attributes[args[1]] = jsonMarshaler - continue - } - strAttr, ok := fieldValue.Interface().(string) if ok { node.Attributes[args[1]] = strAttr diff --git a/response_test.go b/response_test.go index d38977c1..8c96cfb8 100644 --- a/response_test.go +++ b/response_test.go @@ -1236,17 +1236,12 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { { type Model struct { - Thing `jsonapi:"-"` - ModelID int `jsonapi:"primary,models"` - Foo string `jsonapi:"attr,foo"` - Bar string `jsonapi:"attr,bar"` - Bat string `jsonapi:"attr,bat"` - Buzz int `jsonapi:"attr,buzz"` - CreateDate ISO8601Datetime `jsonapi:"attr,create-date"` - } - - isoDate := ISO8601Datetime{ - Time: time.Date(2016, time.December, 8, 15, 18, 54, 0, time.UTC), + Thing `jsonapi:"-"` + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + Buzz int `jsonapi:"attr,buzz"` } scenarios = append(scenarios, test{ @@ -1257,21 +1252,19 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { Type: "models", ID: "1", Attributes: map[string]interface{}{ - "bar": "barry", - "bat": "batty", - "buzz": 99, - "foo": "fooey", - "create-date": isoDate.String(), + "bar": "barry", + "bat": "batty", + "buzz": 99, + "foo": "fooey", }, }, }, expected: &Model{ - ModelID: 1, - Foo: "fooey", - Bar: "barry", - Bat: "batty", - Buzz: 99, - CreateDate: isoDate, + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + Buzz: 99, }, }) }