From 53197ad312f527a84ef05a05703df34584cd4a77 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 3 Feb 2017 17:53:23 -0800 Subject: [PATCH 01/28] 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 23288d3..d5cd0ea 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 a58488c..39bf0d2 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 fe29706..0914a01 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 76c3a3d..ee70d08 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 0000000..51e3bf0 --- /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/28] 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 51e3bf0..0d997ca 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/28] 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 0d997ca..0000000 --- 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 71589dc..9fa9865 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/28] move private funcs to bottom --- response.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/response.go b/response.go index ee70d08..0824fcf 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/28] ErrInvalidType should ignore interfaces --- request.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/request.go b/request.go index 0914a01..05bed44 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/28] 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 39bf0d2..6152ba4 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 9fa9865..aac265f 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/28] 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 05bed44..332e426 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 0824fcf..0f34df2 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/28] 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 332e426..2915138 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 aac265f..f191904 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/28] 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 6152ba4..73b7d59 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 2915138..706f4a7 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/28] 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 706f4a7..fa7786f 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/28] add test uses annotationIgnore --- response_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/response_test.go b/response_test.go index f191904..62c16db 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/28] 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 0000000..8e0f4c3 --- /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 0000000..b60b68b --- /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 fa7786f..93ad1d9 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 0f34df2..8ce1f2a 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 62c16db..7380b19 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/28] 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 7380b19..461c70a 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/28] 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 93ad1d9..fda1414 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 8ce1f2a..e73e452 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 461c70a..d38977c 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 7f51be8c13c8e2d89bee6fd3d8996cbe82d7767d Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Tue, 25 Jul 2017 12:49:41 -0700 Subject: [PATCH 15/28] add support for slice of json.Marshaler/Unmarshaler; add UnixMilli type --- attributes.go | 43 +++++++++++++++++++++++++++++++++++++++++++ attributes_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ request.go | 44 ++++++++++++++++---------------------------- response_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 28 deletions(-) diff --git a/attributes.go b/attributes.go index 8e0f4c3..91805ea 100644 --- a/attributes.go +++ b/attributes.go @@ -38,6 +38,34 @@ func (t ISO8601Datetime) String() string { return t.Format(iso8601Layout) } +// UnixMilli (Unix Millisecond) marshals/unmarshals the number of milliseconds elapsed since January 1, 1970 UTC +type UnixMilli struct { + time.Time +} + +// MarshalJSON implements the json.Marshaler interface. +func (t *UnixMilli) MarshalJSON() ([]byte, error) { + return json.Marshal(t.UnixNano() / int64(time.Millisecond)) +} + +// UnmarshalJSON implements the json.Unmarshaler interface. +func (t *UnixMilli) UnmarshalJSON(data []byte) error { + // Ignore null, like in the main JSON package. + s := string(data) + if s == "null" { + return nil + } + + v, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return err + } + + t.Time = time.Unix(v/1000, (v % 1000 * int64(time.Millisecond))).In(time.UTC) + + return nil +} + // func to help determine json.Marshaler implementation // checks both pointer and non-pointer implementations func isJSONMarshaler(fv reflect.Value) (json.Marshaler, bool) { @@ -53,6 +81,11 @@ func isJSONMarshaler(fv reflect.Value) (json.Marshaler, bool) { return u, ok } +func doesImplementJSONUnmarshaler(fv reflect.Value) bool { + _, ok := isJSONUnmarshaler(fv) + return (ok || isSliceOfJSONUnmarshaler(fv)) +} + // func to help determine json.Unmarshaler implementation // checks both pointer and non-pointer implementations func isJSONUnmarshaler(fv reflect.Value) (json.Unmarshaler, bool) { @@ -67,3 +100,13 @@ func isJSONUnmarshaler(fv reflect.Value) (json.Unmarshaler, bool) { u, ok := fv.Addr().Interface().(json.Unmarshaler) return u, ok } + +func isSliceOfJSONUnmarshaler(fv reflect.Value) bool { + if fv.Kind() != reflect.Slice { + return false + } + + typ := reflect.TypeOf(fv.Interface()).Elem() + _, ok := isJSONUnmarshaler(reflect.New(typ)) + return ok +} diff --git a/attributes_test.go b/attributes_test.go index b60b68b..e7c9cfc 100644 --- a/attributes_test.go +++ b/attributes_test.go @@ -58,6 +58,48 @@ func TestISO8601Datetime(t *testing.T) { } } +func TestUnixMilli(t *testing.T) { + type test struct { + stringVal string + dtVal UnixMilli + } + + tests := []*test{ + &test{ + stringVal: "1257894000000", + dtVal: UnixMilli{Time: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC)}, + }, + &test{ + stringVal: "1257894000999", + dtVal: UnixMilli{Time: time.Date(2009, time.November, 10, 23, 0, 0, 999000000, time.UTC)}, + }, + } + + for _, test := range tests { + // unmarshal stringVal by calling UnmarshalJSON() + dt := &UnixMilli{} + 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{} diff --git a/request.go b/request.go index fda1414..bdcb0e6 100644 --- a/request.go +++ b/request.go @@ -535,6 +535,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 doesImplementJSONUnmarshaler(fieldValue) { + return handleJSONUnmarshalerAttributes(data, args, fieldValue) + } + // JSON value was a float (numeric) if v.Kind() == reflect.Float64 { floatValue := v.Interface().(float64) @@ -624,11 +629,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,33 +641,21 @@ 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 - } +func handleJSONUnmarshalerAttributes(data *Node, args []string, fieldValue reflect.Value) error { + v := fieldValue.Addr().Interface() - if err := jsonUnmarshaler.UnmarshalJSON(b); err != nil { - return ok, err - } + b, err := json.Marshal(data.Attributes[args[1]]) + if err != nil { + return err + } - // success; clear value - delete(data.Attributes, args[1]) - return ok, nil + if err := json.Unmarshal(b, v); err != nil { + return err } - // field does not implement any methods, including json.Unmarshaler; continue - return false, nil + // success; clear value + delete(data.Attributes, args[1]) + return nil } func fullNode(n *Node, included *map[string]*Node) *Node { diff --git a/response_test.go b/response_test.go index d38977c..33ae7b1 100644 --- a/response_test.go +++ b/response_test.go @@ -1412,6 +1412,49 @@ 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"` + FunTimes []UnixMilli `jsonapi:"attr,fun-times"` + CreateDate *UnixMilli `jsonapi:"attr,create-date"` + } + + unixMs := UnixMilli{ + Time: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC), + } + + scenarios = append(scenarios, test{ + name: "UnixMilli", + dst: &Model{}, + payload: &OnePayload{ + Data: &Node{ + Type: "models", + ID: "1", + Attributes: map[string]interface{}{ + "bar": "barry", + "bat": "batty", + "foo": "fooey", + "fun-times": []int64{1257894000000, 1257894000000}, + "create-date": 1257894000000, + }, + }, + }, + expected: &Model{ + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + FunTimes: []UnixMilli{unixMs, unixMs}, + CreateDate: &unixMs, + }, + }) + } + for _, scenario := range scenarios { t.Logf("running scenario: %s\n", scenario.name) From fc52cdf58cdd87b20ef36be6b33065a072f7a4b1 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Tue, 25 Jul 2017 16:05:21 -0700 Subject: [PATCH 16/28] add support for maps of json.Marshaler/Unmarshaler --- attributes.go | 22 ++++++++++++++---- response_test.go | 60 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/attributes.go b/attributes.go index 91805ea..58a9dba 100644 --- a/attributes.go +++ b/attributes.go @@ -7,6 +7,10 @@ import ( "time" ) +// NOTE: reciever for MarshalJSON() should not be a pointer +// https://play.golang.org/p/Cf9yYLIzJA (MarshalJSON() w/ pointer reciever) +// https://play.golang.org/p/5EsItAtgXy (MarshalJSON() w/o pointer reciever) + const iso8601Layout = "2006-01-02T15:04:05Z07:00" // ISO8601Datetime represents a ISO8601 formatted datetime @@ -16,7 +20,7 @@ type ISO8601Datetime struct { } // MarshalJSON implements the json.Marshaler interface. -func (t *ISO8601Datetime) MarshalJSON() ([]byte, error) { +func (t ISO8601Datetime) MarshalJSON() ([]byte, error) { s := t.Time.Format(iso8601Layout) return json.Marshal(s) } @@ -44,7 +48,7 @@ type UnixMilli struct { } // MarshalJSON implements the json.Marshaler interface. -func (t *UnixMilli) MarshalJSON() ([]byte, error) { +func (t UnixMilli) MarshalJSON() ([]byte, error) { return json.Marshal(t.UnixNano() / int64(time.Millisecond)) } @@ -83,7 +87,7 @@ func isJSONMarshaler(fv reflect.Value) (json.Marshaler, bool) { func doesImplementJSONUnmarshaler(fv reflect.Value) bool { _, ok := isJSONUnmarshaler(fv) - return (ok || isSliceOfJSONUnmarshaler(fv)) + return (ok || isSliceOfJSONUnmarshaler(fv) || isMapOfJSONUnmarshaler(fv)) } // func to help determine json.Unmarshaler implementation @@ -107,6 +111,16 @@ func isSliceOfJSONUnmarshaler(fv reflect.Value) bool { } typ := reflect.TypeOf(fv.Interface()).Elem() - _, ok := isJSONUnmarshaler(reflect.New(typ)) + _, ok := isJSONUnmarshaler(reflect.Indirect(reflect.New(typ))) + return ok +} + +func isMapOfJSONUnmarshaler(fv reflect.Value) bool { + if fv.Kind() != reflect.Map { + return false + } + + typ := reflect.TypeOf(fv.Interface()).Elem() + _, ok := isJSONUnmarshaler(reflect.Indirect(reflect.New(typ))) return ok } diff --git a/response_test.go b/response_test.go index 33ae7b1..c087b2e 100644 --- a/response_test.go +++ b/response_test.go @@ -1416,12 +1416,16 @@ 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"` - FunTimes []UnixMilli `jsonapi:"attr,fun-times"` - CreateDate *UnixMilli `jsonapi:"attr,create-date"` + ModelID int `jsonapi:"primary,models"` + Foo string `jsonapi:"attr,foo"` + Bar string `jsonapi:"attr,bar"` + Bat string `jsonapi:"attr,bat"` + FunTimes []UnixMilli `jsonapi:"attr,fun-times"` + SadTimes []*UnixMilli `jsonapi:"attr,sad-times"` + GoodTimes map[string]UnixMilli `jsonapi:"attr,good-times"` + BadTimes map[string]*UnixMilli `jsonapi:"attr,bad-times"` + CreateDate *UnixMilli `jsonapi:"attr,create-date"` + UpdateDate UnixMilli `jsonapi:"attr,update-date"` } unixMs := UnixMilli{ @@ -1429,28 +1433,48 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { } scenarios = append(scenarios, test{ - name: "UnixMilli", + name: "UnixMilli in all supported variations", dst: &Model{}, payload: &OnePayload{ Data: &Node{ Type: "models", ID: "1", Attributes: map[string]interface{}{ - "bar": "barry", - "bat": "batty", - "foo": "fooey", - "fun-times": []int64{1257894000000, 1257894000000}, + "bar": "barry", + "bat": "batty", + "foo": "fooey", + "fun-times": []int64{1257894000000, 1257894000000}, + "sad-times": []int64{1257894000000, 1257894000000}, + "bad-times": map[string]int64{ + "abc": 1257894000000, + "xyz": 1257894000000, + }, + "good-times": map[string]int64{ + "abc": 1257894000000, + "xyz": 1257894000000, + }, "create-date": 1257894000000, + "update-date": 1257894000000, }, }, }, expected: &Model{ - ModelID: 1, - Foo: "fooey", - Bar: "barry", - Bat: "batty", - FunTimes: []UnixMilli{unixMs, unixMs}, + ModelID: 1, + Foo: "fooey", + Bar: "barry", + Bat: "batty", + FunTimes: []UnixMilli{unixMs, unixMs}, + SadTimes: []*UnixMilli{&unixMs, &unixMs}, + GoodTimes: map[string]UnixMilli{ + "abc": unixMs, + "xyz": unixMs, + }, + BadTimes: map[string]*UnixMilli{ + "abc": &unixMs, + "xyz": &unixMs, + }, CreateDate: &unixMs, + UpdateDate: unixMs, }, }) } @@ -1476,7 +1500,7 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { t.Fatal(err) } if !isJSONEqual { - t.Errorf("Got\n%s\nExpected\n%s\n", buf.Bytes(), payload) + t.Errorf("Marshaling Got\n%s\nExpected\n%s\n", buf.Bytes(), payload) } // run jsonapi unmarshal @@ -1486,7 +1510,7 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { // 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) + t.Errorf("Unmarshaling Got\n%#v\nExpected\n%#v\n", scenario.dst, scenario.expected) } } } From 405fe10b91e5ee49d89efd4578de653feca8073f Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Tue, 25 Jul 2017 16:21:12 -0700 Subject: [PATCH 17/28] additional tests for marshal/unmarshal behavior of custom types --- attributes_test.go | 127 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/attributes_test.go b/attributes_test.go index e7c9cfc..1773fbe 100644 --- a/attributes_test.go +++ b/attributes_test.go @@ -1,6 +1,7 @@ package jsonapi import ( + "encoding/json" "reflect" "strconv" "testing" @@ -58,6 +59,132 @@ func TestISO8601Datetime(t *testing.T) { } } +func TestUnixMilliVariations(t *testing.T) { + control := UnixMilli{ + Time: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC), + } + + { + var val map[string]UnixMilli + t.Logf("\nval: %#v\n", val) + + payload := []byte(`{"foo": 1257894000000, "bar":1257894000000}`) + json.Unmarshal(payload, &val) + + if !val["foo"].Time.Equal(control.Time) { + t.Errorf("\n\tE=%+v\n\tA=%+v", control.Time, val["foo"].Time) + } + + b, _ := json.Marshal(val) + is, err := isJSONEqual(b, payload) + if err != nil { + t.Fatal(err) + } + if !is { + t.Errorf("\n\tE=%s\n\tA=%s", payload, b) + } + } + { + var val map[string]*UnixMilli + t.Logf("\nval: %#v\n", val) + + payload := []byte(`{"foo": 1257894000000, "bar":1257894000000}`) + json.Unmarshal(payload, &val) + + if !val["foo"].Time.Equal(control.Time) { + t.Errorf("\n\tE=%+v\n\tA=%+v", control.Time, val["foo"].Time) + } + + b, _ := json.Marshal(val) + is, err := isJSONEqual(b, payload) + if err != nil { + t.Fatal(err) + } + if !is { + t.Errorf("\n\tE=%s\n\tA=%s", payload, b) + } + } + { + var val []*UnixMilli + t.Logf("\nval: %#v\n", val) + + payload := []byte(`[1257894000000,1257894000000]`) + json.Unmarshal(payload, &val) + + if !val[0].Time.Equal(control.Time) { + t.Errorf("\n\tE=%+v\n\tA=%+v", control.Time, val[0].Time) + } + + b, _ := json.Marshal(val) + is, err := isJSONEqual(b, payload) + if err != nil { + t.Fatal(err) + } + if !is { + t.Errorf("\n\tE=%s\n\tA=%s", payload, b) + } + } + { + var val []UnixMilli + t.Logf("\nval: %#v\n", val) + + payload := []byte(`[1257894000000,1257894000000]`) + json.Unmarshal(payload, &val) + + if !val[0].Time.Equal(control.Time) { + t.Errorf("\n\tE=%+v\n\tA=%+v", control.Time, val[0].Time) + } + + b, _ := json.Marshal(val) + is, err := isJSONEqual(b, payload) + if err != nil { + t.Fatal(err) + } + if !is { + t.Errorf("\n\tE=%s\n\tA=%s", payload, b) + } + } + { + var val UnixMilli + t.Logf("\nval: %#v\n", val) + + payload := []byte(`1257894000000`) + json.Unmarshal(payload, &val) + + if !val.Time.Equal(control.Time) { + t.Errorf("\n\tE=%+v\n\tA=%+v", control.Time, val.Time) + } + + b, _ := json.Marshal(val) + is, err := isJSONEqual(b, payload) + if err != nil { + t.Fatal(err) + } + if !is { + t.Errorf("\n\tE=%s\n\tA=%s", payload, b) + } + } + { + var val *UnixMilli + t.Logf("\nval: %#v\n", val) + + payload := []byte(`1257894000000`) + json.Unmarshal(payload, &val) + + if !val.Time.Equal(control.Time) { + t.Errorf("\n\tE=%+v\n\tA=%+v", control.Time, val.Time) + } + + b, _ := json.Marshal(val) + is, err := isJSONEqual(b, payload) + if err != nil { + t.Fatal(err) + } + if !is { + t.Errorf("\n\tE=%s\n\tA=%s", payload, b) + } + } +} func TestUnixMilli(t *testing.T) { type test struct { stringVal string From 3aaeac86cb7166a97bd1ea5eadcf821d32d72e62 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Wed, 26 Jul 2017 11:18:36 -0700 Subject: [PATCH 18/28] add float/exponential notation support for UnixMilli --- attributes.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/attributes.go b/attributes.go index 58a9dba..d3ec6f5 100644 --- a/attributes.go +++ b/attributes.go @@ -4,6 +4,7 @@ import ( "encoding/json" "reflect" "strconv" + "strings" "time" ) @@ -60,9 +61,22 @@ func (t *UnixMilli) UnmarshalJSON(data []byte) error { return nil } - v, err := strconv.ParseInt(s, 10, 64) - if err != nil { - return err + // https://golang.org/doc/go1.8#encoding_json + // go1.8 prefers decimal notation + // go1.7 may use exponetial notation, so check if it came in as a float + var v int64 + if strings.Contains(s, ".") { + fv, err := strconv.ParseFloat(s, 64) + if err != nil { + return err + } + v = int64(fv) + } else { + iv, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return err + } + v = iv } t.Time = time.Unix(v/1000, (v % 1000 * int64(time.Millisecond))).In(time.UTC) From ddc78e5b1fe8fbe8d72c48a665b35fab68226c38 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 28 Jul 2017 10:25:00 -0700 Subject: [PATCH 19/28] fix TestEmbededStructs_nonNilStructPtr; bug on loop w/ (multiple) embedded structs; should just continue on success --- request.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/request.go b/request.go index 9e0eb1a..e962943 100644 --- a/request.go +++ b/request.go @@ -218,10 +218,12 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) assign(em.structField, tmp) data = copy } - return nil + } else { + // handle non-nil scenarios + if err := unmarshalNode(data, em.model, included); err != nil { + return err + } } - // handle non-nil scenarios - return unmarshalNode(data, em.model, included) } return nil From 01f918e4a4758dc75e000cff604fc83045aea8c6 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 28 Jul 2017 10:30:34 -0700 Subject: [PATCH 20/28] fix TestMarshal_duplicatePrimaryAnnotationFromEmbeddedStructs; fix order of processing of visitModelNode --- response.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/response.go b/response.go index 2e9acd7..a19848a 100644 --- a/response.go +++ b/response.go @@ -202,8 +202,10 @@ func MarshalOnePayloadEmbedded(w io.Writer, model interface{}) error { return nil } -func visitModelNode(model interface{}, included *map[string]*Node, - sideload bool) (*Node, error) { +// visitModelNode converts models to jsonapi payloads +// it handles the deepest models first. (i.e.) embedded models +// this is so that upper-level attributes can overwrite lower-level attributes +func visitModelNode(model interface{}, included *map[string]*Node, sideload bool) (*Node, error) { node := new(Node) var er error @@ -211,12 +213,13 @@ func visitModelNode(model interface{}, included *map[string]*Node, modelValue := reflect.ValueOf(model).Elem() modelType := reflect.ValueOf(model).Type().Elem() + // handle just the embedded models first for i := 0; i < modelValue.NumField(); i++ { fieldValue := modelValue.Field(i) fieldType := modelType.Field(i) + // skip if annotated w/ ignore tag := fieldType.Tag.Get(annotationJSONAPI) - if shouldIgnoreField(tag) { continue } @@ -239,6 +242,22 @@ func visitModelNode(model interface{}, included *map[string]*Node, break } node.merge(embNode) + } + } + + // handle everthing else + for i := 0; i < modelValue.NumField(); i++ { + fieldValue := modelValue.Field(i) + fieldType := modelType.Field(i) + + tag := fieldType.Tag.Get(annotationJSONAPI) + + if shouldIgnoreField(tag) { + continue + } + + // skip embedded because it was handled in a previous loop + if isEmbeddedStruct(fieldType) || isEmbeddedStructPtr(fieldType) { continue } From d874c215c7669dd05fd34af020b7b34033bfaf26 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 28 Jul 2017 11:10:58 -0700 Subject: [PATCH 21/28] make ISO8601Datetime and UnixMilli private --- attributes.go | 20 ++++++++++---------- attributes_test.go | 38 +++++++++++++++++++------------------- response_test.go | 28 ++++++++++++++-------------- 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/attributes.go b/attributes.go index d3ec6f5..0f587fb 100644 --- a/attributes.go +++ b/attributes.go @@ -14,20 +14,20 @@ import ( const iso8601Layout = "2006-01-02T15:04:05Z07:00" -// ISO8601Datetime represents a ISO8601 formatted datetime +// iso8601Datetime represents a ISO8601 formatted datetime // It is a time.Time instance that marshals and unmarshals to the ISO8601 ref -type ISO8601Datetime struct { +type iso8601Datetime struct { time.Time } // MarshalJSON implements the json.Marshaler interface. -func (t ISO8601Datetime) MarshalJSON() ([]byte, error) { +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 { +func (t *iso8601Datetime) UnmarshalJSON(data []byte) error { // Ignore null, like in the main JSON package. if string(data) == "null" { return nil @@ -38,23 +38,23 @@ func (t *ISO8601Datetime) UnmarshalJSON(data []byte) error { return err } -// ISO8601Datetime.String() - override default String() on time -func (t ISO8601Datetime) String() string { +// iso8601Datetime.String() - override default String() on time +func (t iso8601Datetime) String() string { return t.Format(iso8601Layout) } -// UnixMilli (Unix Millisecond) marshals/unmarshals the number of milliseconds elapsed since January 1, 1970 UTC -type UnixMilli struct { +// unixMilli (Unix Millisecond) marshals/unmarshals the number of milliseconds elapsed since January 1, 1970 UTC +type unixMilli struct { time.Time } // MarshalJSON implements the json.Marshaler interface. -func (t UnixMilli) MarshalJSON() ([]byte, error) { +func (t unixMilli) MarshalJSON() ([]byte, error) { return json.Marshal(t.UnixNano() / int64(time.Millisecond)) } // UnmarshalJSON implements the json.Unmarshaler interface. -func (t *UnixMilli) UnmarshalJSON(data []byte) error { +func (t *unixMilli) UnmarshalJSON(data []byte) error { // Ignore null, like in the main JSON package. s := string(data) if s == "null" { diff --git a/attributes_test.go b/attributes_test.go index 1773fbe..bc7f7cf 100644 --- a/attributes_test.go +++ b/attributes_test.go @@ -8,7 +8,7 @@ import ( "time" ) -func TestISO8601Datetime(t *testing.T) { +func TestIso8601Datetime(t *testing.T) { pacific, err := time.LoadLocation("America/Los_Angeles") if err != nil { t.Fatal(err) @@ -16,27 +16,27 @@ func TestISO8601Datetime(t *testing.T) { type test struct { stringVal string - dtVal ISO8601Datetime + 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)}, + 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)}, + 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)}, + 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{} + dt := &iso8601Datetime{} if err := dt.UnmarshalJSON([]byte(test.stringVal)); err != nil { t.Fatal(err) } @@ -60,12 +60,12 @@ func TestISO8601Datetime(t *testing.T) { } func TestUnixMilliVariations(t *testing.T) { - control := UnixMilli{ + control := unixMilli{ Time: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC), } { - var val map[string]UnixMilli + var val map[string]unixMilli t.Logf("\nval: %#v\n", val) payload := []byte(`{"foo": 1257894000000, "bar":1257894000000}`) @@ -85,7 +85,7 @@ func TestUnixMilliVariations(t *testing.T) { } } { - var val map[string]*UnixMilli + var val map[string]*unixMilli t.Logf("\nval: %#v\n", val) payload := []byte(`{"foo": 1257894000000, "bar":1257894000000}`) @@ -105,7 +105,7 @@ func TestUnixMilliVariations(t *testing.T) { } } { - var val []*UnixMilli + var val []*unixMilli t.Logf("\nval: %#v\n", val) payload := []byte(`[1257894000000,1257894000000]`) @@ -125,7 +125,7 @@ func TestUnixMilliVariations(t *testing.T) { } } { - var val []UnixMilli + var val []unixMilli t.Logf("\nval: %#v\n", val) payload := []byte(`[1257894000000,1257894000000]`) @@ -145,7 +145,7 @@ func TestUnixMilliVariations(t *testing.T) { } } { - var val UnixMilli + var val unixMilli t.Logf("\nval: %#v\n", val) payload := []byte(`1257894000000`) @@ -165,7 +165,7 @@ func TestUnixMilliVariations(t *testing.T) { } } { - var val *UnixMilli + var val *unixMilli t.Logf("\nval: %#v\n", val) payload := []byte(`1257894000000`) @@ -188,23 +188,23 @@ func TestUnixMilliVariations(t *testing.T) { func TestUnixMilli(t *testing.T) { type test struct { stringVal string - dtVal UnixMilli + dtVal unixMilli } tests := []*test{ &test{ stringVal: "1257894000000", - dtVal: UnixMilli{Time: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC)}, + dtVal: unixMilli{Time: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC)}, }, &test{ stringVal: "1257894000999", - dtVal: UnixMilli{Time: time.Date(2009, time.November, 10, 23, 0, 0, 999000000, time.UTC)}, + dtVal: unixMilli{Time: time.Date(2009, time.November, 10, 23, 0, 0, 999000000, time.UTC)}, }, } for _, test := range tests { // unmarshal stringVal by calling UnmarshalJSON() - dt := &UnixMilli{} + dt := &unixMilli{} if err := dt.UnmarshalJSON([]byte(test.stringVal)); err != nil { t.Fatal(err) } @@ -229,7 +229,7 @@ func TestUnixMilli(t *testing.T) { func TestIsJSONMarshaler(t *testing.T) { { // positive - isoDateTime := ISO8601Datetime{} + isoDateTime := iso8601Datetime{} v := reflect.ValueOf(&isoDateTime) if _, ok := isJSONMarshaler(v); !ok { t.Error("got false; expected ISO8601Datetime to implement json.Marshaler") @@ -247,7 +247,7 @@ func TestIsJSONMarshaler(t *testing.T) { func TestIsJSONUnmarshaler(t *testing.T) { { // positive - isoDateTime := ISO8601Datetime{} + isoDateTime := iso8601Datetime{} v := reflect.ValueOf(&isoDateTime) if _, ok := isJSONUnmarshaler(v); !ok { t.Error("expected ISO8601Datetime to implement json.Unmarshaler") diff --git a/response_test.go b/response_test.go index 2391e57..3cb4893 100644 --- a/response_test.go +++ b/response_test.go @@ -1242,10 +1242,10 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { Bar string `jsonapi:"attr,bar"` Bat string `jsonapi:"attr,bat"` Buzz int `jsonapi:"attr,buzz"` - CreateDate ISO8601Datetime `jsonapi:"attr,create-date"` + CreateDate iso8601Datetime `jsonapi:"attr,create-date"` } - isoDate := ISO8601Datetime{ + isoDate := iso8601Datetime{ Time: time.Date(2016, time.December, 8, 15, 18, 54, 0, time.UTC), } @@ -1420,20 +1420,20 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { Foo string `jsonapi:"attr,foo"` Bar string `jsonapi:"attr,bar"` Bat string `jsonapi:"attr,bat"` - FunTimes []UnixMilli `jsonapi:"attr,fun-times"` - SadTimes []*UnixMilli `jsonapi:"attr,sad-times"` - GoodTimes map[string]UnixMilli `jsonapi:"attr,good-times"` - BadTimes map[string]*UnixMilli `jsonapi:"attr,bad-times"` - CreateDate *UnixMilli `jsonapi:"attr,create-date"` - UpdateDate UnixMilli `jsonapi:"attr,update-date"` + FunTimes []unixMilli `jsonapi:"attr,fun-times"` + SadTimes []*unixMilli `jsonapi:"attr,sad-times"` + GoodTimes map[string]unixMilli `jsonapi:"attr,good-times"` + BadTimes map[string]*unixMilli `jsonapi:"attr,bad-times"` + CreateDate *unixMilli `jsonapi:"attr,create-date"` + UpdateDate unixMilli `jsonapi:"attr,update-date"` } - unixMs := UnixMilli{ + unixMs := unixMilli{ Time: time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC), } scenarios = append(scenarios, test{ - name: "UnixMilli in all supported variations", + name: "unixMilli in all supported variations", dst: &Model{}, payload: &OnePayload{ Data: &Node{ @@ -1463,13 +1463,13 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { Foo: "fooey", Bar: "barry", Bat: "batty", - FunTimes: []UnixMilli{unixMs, unixMs}, - SadTimes: []*UnixMilli{&unixMs, &unixMs}, - GoodTimes: map[string]UnixMilli{ + FunTimes: []unixMilli{unixMs, unixMs}, + SadTimes: []*unixMilli{&unixMs, &unixMs}, + GoodTimes: map[string]unixMilli{ "abc": unixMs, "xyz": unixMs, }, - BadTimes: map[string]*UnixMilli{ + BadTimes: map[string]*unixMilli{ "abc": &unixMs, "xyz": &unixMs, }, From 75748b16dd7d863925f898a7bb662ecaa5e5b66c Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 28 Jul 2017 15:46:56 -0700 Subject: [PATCH 22/28] clean up comment --- response.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/response.go b/response.go index 7cd5c64..3e11b17 100644 --- a/response.go +++ b/response.go @@ -203,7 +203,7 @@ func MarshalOnePayloadEmbedded(w io.Writer, model interface{}) error { } // visitModelNode converts models to jsonapi payloads -// it handles the deepest models first. (i.e.) embedded models +// it handles the deepest models first. (i.e. embedded models) // this is so that upper-level attributes can overwrite lower-level attributes func visitModelNode(model interface{}, included *map[string]*Node, sideload bool) (*Node, error) { node := new(Node) From 1a042c565d263ccf7f5e59adadc9f3705fa0ac29 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 28 Jul 2017 15:47:20 -0700 Subject: [PATCH 23/28] simplify and refactor handleAttributeUnmarshal() --- attributes.go | 75 +++++++++--- request.go | 295 ++++++++++++++++-------------------------------- request_test.go | 43 ++++++- 3 files changed, 191 insertions(+), 222 deletions(-) diff --git a/attributes.go b/attributes.go index 0f587fb..c674260 100644 --- a/attributes.go +++ b/attributes.go @@ -34,7 +34,9 @@ func (t *iso8601Datetime) UnmarshalJSON(data []byte) error { } // Fractional seconds are handled implicitly by Parse. var err error - t.Time, err = time.Parse(strconv.Quote(iso8601Layout), string(data)) + if t.Time, err = time.Parse(strconv.Quote(iso8601Layout), string(data)); err != nil { + return ErrInvalidISO8601 + } return err } @@ -43,6 +45,36 @@ func (t iso8601Datetime) String() string { return t.Format(iso8601Layout) } +// unix(Unix Seconds) marshals/unmarshals the number of milliseconds elapsed since January 1, 1970 UTC +type unix struct { + time.Time +} + +// MarshalJSON implements the json.Marshaler interface. +func (t unix) MarshalJSON() ([]byte, error) { + return json.Marshal(t.Unix()) +} + +// UnmarshalJSON implements the json.Unmarshaler interface. +func (t *unix) UnmarshalJSON(data []byte) error { + // Ignore null, like in the main JSON package. + s := string(data) + if s == "null" { + return nil + } + + v, err := stringToInt64(s) + if err != nil { + // return this specific error to maintain existing tests. + // TODO: consider refactoring tests to not assert against error string + return ErrInvalidTime + } + + t.Time = time.Unix(v, 0).In(time.UTC) + + return nil +} + // unixMilli (Unix Millisecond) marshals/unmarshals the number of milliseconds elapsed since January 1, 1970 UTC type unixMilli struct { time.Time @@ -61,22 +93,9 @@ func (t *unixMilli) UnmarshalJSON(data []byte) error { return nil } - // https://golang.org/doc/go1.8#encoding_json - // go1.8 prefers decimal notation - // go1.7 may use exponetial notation, so check if it came in as a float - var v int64 - if strings.Contains(s, ".") { - fv, err := strconv.ParseFloat(s, 64) - if err != nil { - return err - } - v = int64(fv) - } else { - iv, err := strconv.ParseInt(s, 10, 64) - if err != nil { - return err - } - v = iv + v, err := stringToInt64(s) + if err != nil { + return err } t.Time = time.Unix(v/1000, (v % 1000 * int64(time.Millisecond))).In(time.UTC) @@ -138,3 +157,25 @@ func isMapOfJSONUnmarshaler(fv reflect.Value) bool { _, ok := isJSONUnmarshaler(reflect.Indirect(reflect.New(typ))) return ok } + +// stringToInt64 convert time in either decimal or exponential notation to int64 +// https://golang.org/doc/go1.8#encoding_json +// go1.8 prefers decimal notation +// go1.7 may use exponetial notation, so check if it came in as a float +func stringToInt64(s string) (int64, error) { + var v int64 + if strings.Contains(s, ".") { + fv, err := strconv.ParseFloat(s, 64) + if err != nil { + return v, err + } + v = int64(fv) + } else { + iv, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return v, err + } + v = iv + } + return v, nil +} diff --git a/request.go b/request.go index 6194fc7..0d57209 100644 --- a/request.go +++ b/request.go @@ -32,6 +32,9 @@ var ( ErrUnsupportedPtrType = errors.New("Pointer type in struct is not supported") // ErrInvalidType is returned when the given type is incompatible with the expected type. ErrInvalidType = errors.New("Invalid type provided") // I wish we used punctuation. + + timeType = reflect.TypeOf(time.Time{}) + ptrTimeType = reflect.TypeOf(new(time.Time)) ) // UnmarshalPayload converts an io into a struct instance using jsonapi tags on @@ -420,7 +423,7 @@ func handleToManyRelationUnmarshal(relationData interface{}, fieldType reflect.T return &models, nil } -// TODO: break this out into smaller funcs +// handleAttributeUnmarshal func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.StructField, fieldValue reflect.Value) error { if len(args) < 2 { return ErrBadJSONAPIStructTag @@ -430,16 +433,6 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc return nil } - var iso8601 bool - - if len(args) > 2 { - for _, arg := range args[2:] { - if arg == annotationISO8601 { - iso8601 = true - } - } - } - val := attributes[args[1]] // continue if the attribute was not included in the request @@ -447,203 +440,83 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc return nil } - v := reflect.ValueOf(val) - - // 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 - } - - t, err := time.Parse(iso8601TimeFormat, tm) - if err != nil { - return ErrInvalidISO8601 - } - - 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 - } - - t := time.Unix(at, 0) - - fieldValue.Set(reflect.ValueOf(t)) - delete(data.Attributes, args[1]) - return nil + // custom handling of time + if isTimeValue(fieldValue) { + return handleTimeAttributes(data, args, fieldValue, fieldType) } - 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) - } - - fieldValue.Set(reflect.ValueOf(values)) - delete(data.Attributes, args[1]) - return nil + // standard attributes that the json package knows how to handle, plus implementions on json.Unmarshaler + if doesImplementJSONUnmarshaler(fieldValue) || hasStandardJSONSupport(fieldType) { + return handleWithJSONMarshaler(data, args, fieldValue) } - if fieldValue.Type() == reflect.TypeOf(new(time.Time)) { - if iso8601 { - var tm string - if v.Kind() == reflect.String { - tm = v.Interface().(string) - } else { - 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 - } - - // TODO: refactor the time type handlers to implement json.Unmarshaler and move this higher - if doesImplementJSONUnmarshaler(fieldValue) { - return handleJSONUnmarshalerAttributes(data, args, fieldValue) + // As a final catch-all, ensure types line up to avoid a runtime panic. + // Ignore interfaces since interfaces are poly + // TODO: might not need the if statement anymore. can just end the func w/ `return ErrInvalidType`? + if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != reflect.ValueOf(val).Kind() { + return ErrInvalidType } - // JSON value was a float (numeric) - if v.Kind() == reflect.Float64 { - floatValue := v.Interface().(float64) + return nil +} - // 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() - } +func fullNode(n *Node, included *map[string]*Node) *Node { + includedKey := fmt.Sprintf("%s,%s", n.Type, n.ID) - var numericValue reflect.Value + if included != nil && (*included)[includedKey] != nil { + return deepCopyNode((*included)[includedKey]) + } - 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 - } + return deepCopyNode(n) +} - assign(fieldValue, numericValue) - delete(data.Attributes, args[1]) - return nil +// assign will take the value specified and assign it to the field; if +// field is expecting a ptr assign will assign a ptr. +func assign(field, value reflect.Value) { + if field.Kind() == reflect.Ptr { + field.Set(value) + } else { + field.Set(reflect.Indirect(value)) } +} - // 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 +// handleTimeAttributes - handle field of type time.Time and *time.Time +// TODO: consider refactoring/removing this toggling (would be a breaking change) +// time.Time implements RFC3339 (https://golang.org/pkg/time/#Time.UnmarshalJSON) but is overridden here. +// jsonapi doesn't specify, but does recommend ISO8601 (http://jsonapi.org/recommendations/#date-and-time-fields) +// IMHO (skimata): just default on recommended ISO8601, and include custom-type support that would handle +// any struct field type that implements the json marshaler/unmarshaler (PR#104) +func handleTimeAttributes(data *Node, args []string, fieldValue reflect.Value, structField reflect.StructField) error { + b, err := json.Marshal(data.Attributes[args[1]]) + if err != nil { + return err + } + var tm time.Time + if useISO8601(args) { + iso := &iso8601Datetime{} + if err := iso.UnmarshalJSON(b); err != nil { + return err } - - if fieldValue.Type() != concreteVal.Type() { - return ErrUnsupportedPtrType + tm = iso.Time + } else { + epoch := &unix{} + if err := epoch.UnmarshalJSON(b); err != nil { + return err } - - fieldValue.Set(concreteVal) - delete(data.Attributes, args[1]) - return nil + tm = epoch.Time } - // 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 + if structField.Type.Kind() == reflect.Ptr { + fieldValue.Set(reflect.ValueOf(&tm)) + } else { + fieldValue.Set(reflect.ValueOf(tm)) } - // set val and clear attribute key so its not processed again - fieldValue.Set(reflect.ValueOf(val)) delete(data.Attributes, args[1]) return nil } -func handleJSONUnmarshalerAttributes(data *Node, args []string, fieldValue reflect.Value) error { +func handleWithJSONMarshaler(data *Node, args []string, fieldValue reflect.Value) error { v := fieldValue.Addr().Interface() b, err := json.Marshal(data.Attributes[args[1]]) @@ -660,22 +533,46 @@ func handleJSONUnmarshalerAttributes(data *Node, args []string, fieldValue refle return nil } -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 deepCopyNode((*included)[includedKey]) +func isTimeValue(fieldValue reflect.Value) bool { + switch fieldValue.Type() { + default: + return false + case timeType, ptrTimeType: + return true } - return deepCopyNode(n) } -// assign will take the value specified and assign it to the field; if -// field is expecting a ptr assign will assign a ptr. -func assign(field, value reflect.Value) { - if field.Kind() == reflect.Ptr { - field.Set(value) - } else { - field.Set(reflect.Indirect(value)) +func hasStandardJSONSupport(structField reflect.StructField) bool { + kind := structField.Type.Kind() + if kind == reflect.Ptr { + kind = structField.Type.Elem().Kind() + } + + switch kind { + default: + return false + case reflect.Map, reflect.Slice: + return hasStandardJSONSupport(reflect.StructField{Type: structField.Type.Elem()}) + case + reflect.Bool, + reflect.String, + reflect.Complex64, reflect.Complex128, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, + reflect.Uintptr, + reflect.Float32, reflect.Float64: + return true + } +} + +func useISO8601(args []string) bool { + if len(args) > 2 { + for _, arg := range args[2:] { + if arg == annotationISO8601 { + return true + } + } } + return false } diff --git a/request_test.go b/request_test.go index ea9a15f..06a0bc0 100644 --- a/request_test.go +++ b/request_test.go @@ -126,16 +126,18 @@ func TestUnmarshalToStructWithPointerAttr_BadType(t *testing.T) { in := map[string]interface{}{ "name": true, // This is the wrong type. } - expectedErrorMessage := ErrUnsupportedPtrType.Error() err := UnmarshalPayload(sampleWithPointerPayload(in), out) if err == nil { t.Fatalf("Expected error due to invalid type.") } - if err.Error() != expectedErrorMessage { - t.Fatalf("Unexpected error message: %s", err.Error()) + + jTypeErr, ok := err.(*json.UnmarshalTypeError) + if !ok { + t.Fatalf("Expected an unmarshal error, got %#v\n", err) } + t.Logf("successfully returned err when unmarshaling a %s, to a %s field\n", jTypeErr.Value, jTypeErr.Type.String()) } func TestStringPointerField(t *testing.T) { @@ -196,8 +198,36 @@ func TestUnmarshalInvalidJSON_BadType(t *testing.T) { BadValue interface{} Error error }{ // The `Field` values here correspond to the `ModelBadTypes` jsonapi fields. - {Field: "string_field", BadValue: 0, Error: ErrUnknownFieldNumberType}, // Expected string. - {Field: "float_field", BadValue: "A string.", Error: ErrInvalidType}, // Expected float64. + {Field: "string_field", BadValue: 0, Error: ErrUnknownFieldNumberType}, // Expected string. + {Field: "float_field", BadValue: "A string.", Error: ErrInvalidType}, // Expected float64. + } + for _, test := range badTypeTests { + t.Run(fmt.Sprintf("Test_%s", test.Field), func(t *testing.T) { + out := new(ModelBadTypes) + in := map[string]interface{}{} + in[test.Field] = test.BadValue + // should not compare err string + // expectedErrorMessage := test.Error.Error() + + err := UnmarshalPayload(samplePayloadWithBadTypes(in), out) + + if err == nil { + t.Fatalf("Expected error due to invalid type.") + } + jTypeErr, ok := err.(*json.UnmarshalTypeError) + if !ok { + t.Fatalf("Expected an unmarshal error, got %#v\n", err) + } + t.Logf("successfully returned err when unmarshaling a %s, to a %s field\n", jTypeErr.Value, jTypeErr.Type.String()) + }) + } +} +func TestUnmarshalInvalidJSON_BadType_Time(t *testing.T) { + var badTypeTests = []struct { + Field string + BadValue interface{} + Error error + }{ // The `Field` values here correspond to the `ModelBadTypes` jsonapi fields. {Field: "time_field", BadValue: "A string.", Error: ErrInvalidTime}, // Expected int64. {Field: "time_ptr_field", BadValue: "A string.", Error: ErrInvalidTime}, // Expected *time / int64. } @@ -213,6 +243,7 @@ func TestUnmarshalInvalidJSON_BadType(t *testing.T) { if err == nil { t.Fatalf("Expected error due to invalid type.") } + if err.Error() != expectedErrorMessage { t.Fatalf("Unexpected error message: %s", err.Error()) } @@ -332,7 +363,7 @@ func TestUnmarshalInvalidISO8601(t *testing.T) { out := new(Timestamp) if err := UnmarshalPayload(in, out); err != ErrInvalidISO8601 { - t.Fatalf("Expected ErrInvalidISO8601, got %v", err) + t.Fatalf("Expected %v, got %v", ErrInvalidISO8601, err) } } From db8ca4ec4903b3d58a39153e777e41192a3c7a34 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Fri, 28 Jul 2017 16:06:59 -0700 Subject: [PATCH 24/28] cleanup comment --- request.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/request.go b/request.go index 0d57209..2898ba2 100644 --- a/request.go +++ b/request.go @@ -432,7 +432,6 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc if attributes == nil || len(data.Attributes) == 0 { return nil } - val := attributes[args[1]] // continue if the attribute was not included in the request @@ -482,10 +481,10 @@ func assign(field, value reflect.Value) { // handleTimeAttributes - handle field of type time.Time and *time.Time // TODO: consider refactoring/removing this toggling (would be a breaking change) -// time.Time implements RFC3339 (https://golang.org/pkg/time/#Time.UnmarshalJSON) but is overridden here. -// jsonapi doesn't specify, but does recommend ISO8601 (http://jsonapi.org/recommendations/#date-and-time-fields) -// IMHO (skimata): just default on recommended ISO8601, and include custom-type support that would handle -// any struct field type that implements the json marshaler/unmarshaler (PR#104) +// standard time.Time implements RFC3339 (https://golang.org/pkg/time/#Time.UnmarshalJSON) but is overridden here. +// jsonapi doesn't specify, but does recommends ISO8601 (http://jsonapi.org/recommendations/#date-and-time-fields) +// IMHO (skimata): just default on recommended ISO8601, all others desired formats +// should implement w/ a custom marshaler/unmarshaler func handleTimeAttributes(data *Node, args []string, fieldValue reflect.Value, structField reflect.StructField) error { b, err := json.Marshal(data.Attributes[args[1]]) if err != nil { From bf72c7a5182fcb66dccf3ddeb78449fbeb3ce985 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Mon, 31 Jul 2017 11:39:43 -0700 Subject: [PATCH 25/28] improve check on json.Unmarshaler implementations --- attributes.go | 90 ++++++++++++++--------------------- attributes_test.go | 115 +++++++++++++++++++++++++++++++++++++-------- request.go | 2 +- response.go | 5 -- 4 files changed, 131 insertions(+), 81 deletions(-) diff --git a/attributes.go b/attributes.go index c674260..f010f7c 100644 --- a/attributes.go +++ b/attributes.go @@ -14,6 +14,10 @@ import ( const iso8601Layout = "2006-01-02T15:04:05Z07:00" +var ( + jsonUnmarshaler = reflect.TypeOf(new(json.Unmarshaler)).Elem() +) + // iso8601Datetime represents a ISO8601 formatted datetime // It is a time.Time instance that marshals and unmarshals to the ISO8601 ref type iso8601Datetime struct { @@ -103,61 +107,6 @@ func (t *unixMilli) UnmarshalJSON(data []byte) error { return nil } -// 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 doesImplementJSONUnmarshaler(fv reflect.Value) bool { - _, ok := isJSONUnmarshaler(fv) - return (ok || isSliceOfJSONUnmarshaler(fv) || isMapOfJSONUnmarshaler(fv)) -} - -// 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 -} - -func isSliceOfJSONUnmarshaler(fv reflect.Value) bool { - if fv.Kind() != reflect.Slice { - return false - } - - typ := reflect.TypeOf(fv.Interface()).Elem() - _, ok := isJSONUnmarshaler(reflect.Indirect(reflect.New(typ))) - return ok -} - -func isMapOfJSONUnmarshaler(fv reflect.Value) bool { - if fv.Kind() != reflect.Map { - return false - } - - typ := reflect.TypeOf(fv.Interface()).Elem() - _, ok := isJSONUnmarshaler(reflect.Indirect(reflect.New(typ))) - return ok -} - // stringToInt64 convert time in either decimal or exponential notation to int64 // https://golang.org/doc/go1.8#encoding_json // go1.8 prefers decimal notation @@ -179,3 +128,34 @@ func stringToInt64(s string) (int64, error) { } return v, nil } + +func implementsJSONUnmarshaler(t reflect.Type) bool { + ok, _ := deepCheckImplementation(t, jsonUnmarshaler) + return ok +} + +func deepCheckImplementation(t, interfaceType reflect.Type) (bool, reflect.Type) { + // check as-is + if t.Implements(interfaceType) { + return true, t + } + + switch t.Kind() { + case reflect.Array, reflect.Chan, reflect.Map, reflect.Ptr, reflect.Slice: + // check ptr implementation + ptrType := reflect.PtrTo(t) + if ptrType.Implements(interfaceType) { + return true, ptrType + } + // since these are reference types, re-check on the element of t + return deepCheckImplementation(t.Elem(), interfaceType) + default: + // check ptr implementation + ptrType := reflect.PtrTo(t) + if ptrType.Implements(interfaceType) { + return true, ptrType + } + // nothing else to check, return false + return false, nil + } +} diff --git a/attributes_test.go b/attributes_test.go index bc7f7cf..04ee7f4 100644 --- a/attributes_test.go +++ b/attributes_test.go @@ -227,38 +227,113 @@ func TestUnixMilli(t *testing.T) { } } -func TestIsJSONMarshaler(t *testing.T) { +func TestImplementsJSONUnmarshaler(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") + raw := json.RawMessage{} + typ := reflect.TypeOf(&raw) + if ok := implementsJSONUnmarshaler(typ); !ok { + t.Error("expected json.RawMessage to implement json.Unmarshaler") } } -} - -func TestIsJSONUnmarshaler(t *testing.T) { { // positive isoDateTime := iso8601Datetime{} - v := reflect.ValueOf(&isoDateTime) - if _, ok := isJSONUnmarshaler(v); !ok { + typ := reflect.TypeOf(&isoDateTime) + if ok := implementsJSONUnmarshaler(typ); !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 { + typ := reflect.TypeOf(&input) + if ok := implementsJSONUnmarshaler(typ); ok { t.Error("got true; expected customString to not implement json.Unmarshaler") } } } + +func TestDeepCheckImplementation(t *testing.T) { + tests := []struct { + name string + input interface{} + }{ + { + name: "concrete ( RawMessage is a reflect.Type of slice)", + input: json.RawMessage{}, + }, + { + name: "RawMessage ptr", + input: &json.RawMessage{}, + }, + { + name: "concrete slice of RawMessage", + input: []json.RawMessage{}, + }, + { + name: "slice of RawMessage ptrs", + input: []*json.RawMessage{}, + }, + { + name: "concrete map of RawMessage", + input: map[string]json.RawMessage{}, + }, + { + name: "map of RawMessage ptrs", + input: map[string]*json.RawMessage{}, + }, + { + name: "map of RawMessage slice", + input: map[string][]json.RawMessage{}, + }, + { + name: "ptr ptr of RawMessage", + input: func() **json.RawMessage { + r := &json.RawMessage{} + return &r + }(), + }, + { + name: "concrete unixMilli (struct)", + input: unixMilli{}, + }, + { + name: "unixMilli ptr", + input: &unixMilli{}, + }, + { + name: "concrete slice of unixMilli", + input: []unixMilli{}, + }, + { + name: "slice of unixMilli ptrs", + input: []*unixMilli{}, + }, + { + name: "concrete map of unixMilli", + input: map[string]unixMilli{}, + }, + { + name: "map of unixMilli ptrs", + input: map[string]*unixMilli{}, + }, + { + name: "map of unixMilli slice", + input: map[string][]unixMilli{}, + }, + { + name: "ptr ptr of unixMilli", + input: func() **unixMilli { + r := &unixMilli{} + return &r + }(), + }, + } + + for _, scenario := range tests { + typ := reflect.TypeOf(scenario.input) + ok, elemType := deepCheckImplementation(typ, jsonUnmarshaler) + if !ok { + t.Errorf("\n\tE=%v\n\tA=%v", typ, elemType) + } + } +} diff --git a/request.go b/request.go index 2898ba2..dde59f0 100644 --- a/request.go +++ b/request.go @@ -445,7 +445,7 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc } // standard attributes that the json package knows how to handle, plus implementions on json.Unmarshaler - if doesImplementJSONUnmarshaler(fieldValue) || hasStandardJSONSupport(fieldType) { + if implementsJSONUnmarshaler(fieldType.Type) || hasStandardJSONSupport(fieldType) { return handleWithJSONMarshaler(data, args, fieldValue) } diff --git a/response.go b/response.go index 3e11b17..1c582f9 100644 --- a/response.go +++ b/response.go @@ -389,11 +389,6 @@ func visitModelNode(model interface{}, included *map[string]*Node, sideload bool 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 From e55ed48dcc7e547b887425a545f0b686d5c73555 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Mon, 31 Jul 2017 14:20:00 -0700 Subject: [PATCH 26/28] cleanup handlePrimaryUnmarshal() --- request.go | 69 +++++++++++++++--------------------------------------- 1 file changed, 19 insertions(+), 50 deletions(-) diff --git a/request.go b/request.go index dde59f0..c2811e4 100644 --- a/request.go +++ b/request.go @@ -274,65 +274,34 @@ func handlePrimaryUnmarshal(data *Node, args []string, fieldType reflect.StructF kind = fieldType.Type.Kind() } - var idValue reflect.Value + switch kind { + default: + // only handle strings and numerics + return ErrBadJSONAPIID + case reflect.String: + assign(fieldValue, reflect.ValueOf(data.ID)) + case + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - // Handle String case - if kind == reflect.String { - // 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) + fv, 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]) - 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 + b, err := json.Marshal(fv) + if err != nil { + return err + } + + v := fieldValue.Addr().Interface() + if err := json.Unmarshal(b, v); err != nil { + return err } } - // 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 } From 37d04eae1db6b84098893c3f49029afabdcafc45 Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Mon, 31 Jul 2017 14:21:05 -0700 Subject: [PATCH 27/28] rename fieldType to structField --- request.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/request.go b/request.go index c2811e4..2cda006 100644 --- a/request.go +++ b/request.go @@ -33,8 +33,8 @@ var ( // ErrInvalidType is returned when the given type is incompatible with the expected type. ErrInvalidType = errors.New("Invalid type provided") // I wish we used punctuation. - timeType = reflect.TypeOf(time.Time{}) ptrTimeType = reflect.TypeOf(new(time.Time)) + timeType = ptrTimeType.Elem() ) // UnmarshalPayload converts an io into a struct instance using jsonapi tags on @@ -141,9 +141,9 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) embeddeds := []*embedded{} for i := 0; i < modelValue.NumField(); i++ { - fieldType := modelType.Field(i) + structField := modelType.Field(i) fieldValue := modelValue.Field(i) - tag := fieldType.Tag.Get(annotationJSONAPI) + tag := structField.Tag.Get(annotationJSONAPI) // handle explicit ignore annotation if shouldIgnoreField(tag) { @@ -151,7 +151,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) } // handles embedded structs - if isEmbeddedStruct(fieldType) { + if isEmbeddedStruct(structField) { embeddeds = append(embeddeds, &embedded{ model: reflect.ValueOf(fieldValue.Addr().Interface()), @@ -162,7 +162,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) } // handles pointers to embedded structs - if isEmbeddedStructPtr(fieldType) { + if isEmbeddedStructPtr(structField) { embeddeds = append(embeddeds, &embedded{ model: reflect.ValueOf(fieldValue.Interface()), @@ -190,11 +190,11 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) return err } case annotationPrimary: - if err := handlePrimaryUnmarshal(data, args, fieldType, fieldValue); err != nil { + if err := handlePrimaryUnmarshal(data, args, structField, fieldValue); err != nil { return err } case annotationAttribute: - if err := handleAttributeUnmarshal(data, args, fieldType, fieldValue); err != nil { + if err := handleAttributeUnmarshal(data, args, structField, fieldValue); err != nil { return err } case annotationRelation: From a6437e5d17a1eda9932865dded191a1bfad5b66d Mon Sep 17 00:00:00 2001 From: Stephan Kim Date: Wed, 16 Aug 2017 14:52:39 -0700 Subject: [PATCH 28/28] remove 'ErrInvalidType' response; allow all json lib supported types go through --- request.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/request.go b/request.go index 2cda006..86ac9ef 100644 --- a/request.go +++ b/request.go @@ -414,18 +414,7 @@ func handleAttributeUnmarshal(data *Node, args []string, fieldType reflect.Struc } // standard attributes that the json package knows how to handle, plus implementions on json.Unmarshaler - if implementsJSONUnmarshaler(fieldType.Type) || hasStandardJSONSupport(fieldType) { - return handleWithJSONMarshaler(data, args, fieldValue) - } - - // As a final catch-all, ensure types line up to avoid a runtime panic. - // Ignore interfaces since interfaces are poly - // TODO: might not need the if statement anymore. can just end the func w/ `return ErrInvalidType`? - if fieldValue.Kind() != reflect.Interface && fieldValue.Kind() != reflect.ValueOf(val).Kind() { - return ErrInvalidType - } - - return nil + return handleWithJSONMarshaler(data, args, fieldValue) } func fullNode(n *Node, included *map[string]*Node) *Node {