From a6ac768a2785a9900d3fb9fb4dcede730dfeea15 Mon Sep 17 00:00:00 2001 From: skimata Date: Thu, 27 Jul 2017 11:16:00 -0700 Subject: [PATCH 01/13] Embedded structs v3 (#100) * working version * fix text * combine test files * move private funcs to bottom * ErrInvalidType should ignore interfaces * replace MarshalOnePayload w/ MarshalPayload; fix bug w/ node merge() * minor tweaks; address a couple comments * decompose unmarshalNode() to smaller funcs; unmarshal should go from top-level to embedded * deep copy the node when passing relation/sideloaded notes to unmarshal() * add some comments and do some additional cleanup * add test uses annotationIgnore * implement support for struct fields that implement json.Marshaler/Unmarshaler * add additional test that compares marshal/unmarshal behavior w/ standard json library * add support for pointer embedded structs * Revert "implement support for struct fields that implement json.Marshaler/Unmarshaler" This reverts commit deeffb78df43b641cb10e125bfdb021deb14bc72. --- constants.go | 1 + node.go | 64 ++++ request.go | 767 +++++++++++++++++++++++++++-------------------- response.go | 47 ++- response_test.go | 638 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 1182 insertions(+), 335 deletions(-) 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..73b7d59 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 && node.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"` @@ -119,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 fe29706..9e0eb1a 100644 --- a/request.go +++ b/request.go @@ -117,6 +117,11 @@ func UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) { return models, nil } +// 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 { @@ -127,416 +132,518 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) modelValue := model.Elem() modelType := model.Type().Elem() - var er error + type embedded struct { + structField, model reflect.Value + } + embeddeds := []*embedded{} for i := 0; i < modelValue.NumField(); i++ { fieldType := modelType.Field(i) - tag := fieldType.Tag.Get("jsonapi") - if tag == "" { + fieldValue := modelValue.Field(i) + tag := fieldType.Tag.Get(annotationJSONAPI) + + // handle explicit ignore annotation + if shouldIgnoreField(tag) { continue } - fieldValue := modelValue.Field(i) - - args := strings.Split(tag, ",") + // handles embedded structs + if isEmbeddedStruct(fieldType) { + embeddeds = append(embeddeds, + &embedded{ + model: reflect.ValueOf(fieldValue.Addr().Interface()), + structField: fieldValue, + }, + ) + continue + } - if len(args) < 1 { - er = ErrBadJSONAPIStructTag - break + // handles pointers to embedded structs + if isEmbeddedStructPtr(fieldType) { + embeddeds = append(embeddeds, + &embedded{ + model: reflect.ValueOf(fieldValue.Interface()), + structField: fieldValue, + }, + ) + continue } - annotation := args[0] + // handle tagless; after handling embedded structs (which could be tagless) + if tag == "" { + continue + } - if (annotation == annotationClientID && len(args) != 1) || - (annotation != annotationClientID && len(args) < 2) { - er = ErrBadJSONAPIStructTag - break + args := strings.Split(tag, annotationSeperator) + // require atleast 1 + if len(args) < 1 { + return ErrBadJSONAPIStructTag } - if annotation == annotationPrimary { - if data.ID == "" { - continue + // args[0] == annotation + switch args[0] { + case annotationClientID: + if err := handleClientIDUnmarshal(data, args, fieldValue); err != nil { + return err } - - // 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 + case annotationPrimary: + if err := handlePrimaryUnmarshal(data, args, fieldType, fieldValue); err != nil { + return err } - - // 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 { - kind = fieldType.Type.Elem().Kind() - } else { - kind = fieldType.Type.Kind() + case annotationAttribute: + if err := handleAttributeUnmarshal(data, args, fieldType, fieldValue); err != nil { + return err } - - // Handle String case - if kind == reflect.String { - assign(fieldValue, v) - continue + case annotationRelation: + if err := handleRelationUnmarshal(data, args, fieldValue, included); err != nil { + return err } + default: + return fmt.Errorf(unsuportedStructTagMsg, args[0]) + } + } - // 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 + // handle embedded last + 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 } - // 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 + // 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) + } - assign(fieldValue, idValue) - } else if annotation == annotationClientID { - if data.ClientID == "" { - continue - } + return nil +} - fieldValue.Set(reflect.ValueOf(data.ClientID)) - } else if annotation == annotationAttribute { - attributes := data.Attributes - if attributes == nil || len(data.Attributes) == 0 { - continue - } +func handleClientIDUnmarshal(data *Node, args []string, fieldValue reflect.Value) error { + if len(args) != 1 { + return ErrBadJSONAPIStructTag + } - var iso8601 bool + if data.ClientID == "" { + return nil + } - if len(args) > 2 { - for _, arg := range args[2:] { - if arg == annotationISO8601 { - iso8601 = true - } - } - } + // set value and clear clientID to denote it's already been processed + fieldValue.Set(reflect.ValueOf(data.ClientID)) + data.ClientID = "" - val := attributes[args[1]] + return nil +} - // continue if the attribute was not included in the request - if val == nil { - continue - } +func handlePrimaryUnmarshal(data *Node, args []string, fieldType reflect.StructField, fieldValue reflect.Value) error { + if len(args) < 2 { + return ErrBadJSONAPIStructTag + } - v := reflect.ValueOf(val) + if data.ID == "" { + return nil + } - // 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 - } + // 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], + ) + } - t, err := time.Parse(iso8601TimeFormat, tm) - if err != nil { - er = ErrInvalidISO8601 - break - } + // 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(t)) + var idValue reflect.Value - continue - } + // 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) + if err != nil { + // Could not convert the value in the "id" attr to a float + return ErrBadJSONAPIID + } - var at int64 + // 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 + } + } - if v.Kind() == reflect.Float64 { - at = int64(v.Interface().(float64)) - } else if v.Kind() == reflect.Int { - at = v.Int() - } else { - return ErrInvalidTime - } + // set value and clear ID to denote it's already been processed + assign(fieldValue, idValue) + data.ID = "" - t := time.Unix(at, 0) + return nil +} - fieldValue.Set(reflect.ValueOf(t)) +func handleRelationUnmarshal(data *Node, args []string, fieldValue reflect.Value, included *map[string]*Node) error { + if len(args) < 2 { + return ErrBadJSONAPIStructTag + } - continue - } + if data.Relationships == nil || data.Relationships[args[1]] == nil { + return nil + } - 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) - } + // to-one relationships + handler := handleToOneRelationUnmarshal + isSlice := fieldValue.Type().Kind() == reflect.Slice + if isSlice { + // to-many relationship + handler = handleToManyRelationUnmarshal + } - fieldValue.Set(reflect.ValueOf(values)) + v, err := handler(data.Relationships[args[1]], fieldValue.Type(), included) + if err != nil { + return err + } + // 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 +} - continue - } +// 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(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 + 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, nil + } - 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 - } + if err := unmarshalNode( + fullNode(relationship.Data, included), + m, + included, + ); err != nil { + return nil, err + } - v, err := time.Parse(iso8601TimeFormat, tm) - if err != nil { - er = ErrInvalidISO8601 - break - } + return &m, nil +} - t := &v +// to-many relationship +func handleToManyRelationUnmarshal(relationData interface{}, fieldType reflect.Type, included *map[string]*Node) (*reflect.Value, error) { + relationship := new(RelationshipManyNode) - fieldValue.Set(reflect.ValueOf(t)) + buf := bytes.NewBuffer(nil) + json.NewEncoder(buf).Encode(relationData) + json.NewDecoder(buf).Decode(relationship) - continue - } + models := reflect.New(fieldType).Elem() - var at int64 + rData := relationship.Data + for _, n := range rData { + m := reflect.New(fieldType.Elem().Elem()) - if v.Kind() == reflect.Float64 { - at = int64(v.Interface().(float64)) - } else if v.Kind() == reflect.Int { - at = v.Int() - } else { - return ErrInvalidTime - } + if err := unmarshalNode( + fullNode(n, included), + m, + included, + ); err != nil { + return nil, err + } - v := time.Unix(at, 0) - t := &v + models = reflect.Append(models, m) + } - fieldValue.Set(reflect.ValueOf(t)) + return &models, nil +} - continue - } +// 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 + } + attributes := data.Attributes + if attributes == nil || len(data.Attributes) == 0 { + 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 { - 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 - } + var iso8601 bool - // 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 + if len(args) > 2 { + for _, arg := range args[2:] { + if arg == annotationISO8601 { + iso8601 = true } + } + } - // As a final catch-all, ensure types line up to avoid a runtime panic. - if fieldValue.Kind() != v.Kind() { - return ErrInvalidType - } - fieldValue.Set(reflect.ValueOf(val)) + val := attributes[args[1]] + + // continue if the attribute was not included in the request + if val == nil { + return nil + } - } else if annotation == annotationRelation { - isSlice := fieldValue.Type().Kind() == reflect.Slice + v := reflect.ValueOf(val) - if data.Relationships == nil || data.Relationships[args[1]] == nil { - 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 } - if isSlice { - // to-many relationship - relationship := new(RelationshipManyNode) + t, err := time.Parse(iso8601TimeFormat, tm) + if err != nil { + return ErrInvalidISO8601 + } - buf := bytes.NewBuffer(nil) + fieldValue.Set(reflect.ValueOf(t)) + delete(data.Attributes, args[1]) + return nil + } + + var at int64 - json.NewEncoder(buf).Encode(data.Relationships[args[1]]) - json.NewDecoder(buf).Decode(relationship) + if v.Kind() == reflect.Float64 { + at = int64(v.Interface().(float64)) + } else if v.Kind() == reflect.Int { + at = v.Int() + } else { + return ErrInvalidTime + } - data := relationship.Data - models := reflect.New(fieldValue.Type()).Elem() + t := time.Unix(at, 0) - for _, n := range data { - m := reflect.New(fieldValue.Type().Elem().Elem()) + fieldValue.Set(reflect.ValueOf(t)) + delete(data.Attributes, args[1]) + 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 } - return er + // set val and clear attribute key so its not processed again + fieldValue.Set(reflect.ValueOf(val)) + delete(data.Attributes, args[1]) + 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 (*included)[includedKey] + return deepCopyNode((*included)[includedKey]) } - return n + return deepCopyNode(n) } // assign will take the value specified and assign it to the field; if diff --git a/response.go b/response.go index 76c3a3d..2e9acd7 100644 --- a/response.go +++ b/response.go @@ -212,14 +212,39 @@ 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) - if tag == "" { + fieldValue := modelValue.Field(i) + fieldType := modelType.Field(i) + + tag := fieldType.Tag.Get(annotationJSONAPI) + + if shouldIgnoreField(tag) { continue } - fieldValue := modelValue.Field(i) - fieldType := modelType.Field(i) + // 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 + } args := strings.Split(tag, annotationSeperator) @@ -533,3 +558,15 @@ func convertToSliceInterface(i *interface{}) ([]interface{}, error) { } return response, nil } + +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 71589dc..8c96cfb 100644 --- a/response_test.go +++ b/response_test.go @@ -817,6 +817,630 @@ 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) + } + } +} + +// 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"` + 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"` + } + + 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 + } + + 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", + }, + }, + }) + } + + { + 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 + } + + 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", + }, + }, + }) + } + + { + 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, + }, + }) + } + { + 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) + + // 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", buf.Bytes(), payload) + } + + // 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) + } + } +} + func testBlog() *Blog { return &Blog{ ID: 5, @@ -883,3 +1507,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 3e612cc977fceb1b45e8544ff05312e54c2c8609 Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Thu, 27 Jul 2017 11:30:09 -0700 Subject: [PATCH 02/13] Added a failing test for the allocation of struct pointers on serialization/deserilization. --- models_test.go | 19 +++++++++++ request_test.go | 91 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/models_test.go b/models_test.go index a53dd61..1059ad4 100644 --- a/models_test.go +++ b/models_test.go @@ -155,3 +155,22 @@ func (bc *BadComment) JSONAPILinks() *Links { "self": []string{"invalid", "should error"}, } } + +// Embeded Struct Models +type Engine struct { + NumberOfCylinders uint `jsonapi:"attr,cylinders"` + HorsePower uint `jsonapi:"attr,hp"` +} + +type BlockHeater struct { + Watts uint `jsonapi:"attr,watts"` +} + +type Vehicle struct { + ID uint `json:"id" jsonapi:"primary,car"` + Make string `jsonapi:"attr,make"` + Model string `jsonapi:"attr,model"` + Year uint `jsonapi:"attr,year"` + Engine // every car must have an engine + *BlockHeater // not every car will have a block heater +} diff --git a/request_test.go b/request_test.go index 6b47fd7..ea9a15f 100644 --- a/request_test.go +++ b/request_test.go @@ -703,6 +703,97 @@ func TestManyPayload_withLinks(t *testing.T) { } } +func TestEmbededStructs_nonNilStructPtr(t *testing.T) { + originalVehicle := &Vehicle{ + Make: "VW", + Model: "R32", + Year: 2008, + Engine: Engine{ + NumberOfCylinders: 6, + HorsePower: 250, + }, + BlockHeater: &BlockHeater{ + Watts: 150, + }, + } + + // Serialize as JSON + jsonVehicle, err := json.Marshal(originalVehicle) + if err != nil { + t.Fatal(err) + } + + jsonUnmarshalledVehicle := &Vehicle{} + json.Unmarshal(jsonVehicle, jsonUnmarshalledVehicle) + + // Proves that the JSON standard lib will allocate a BlockHeater + if jsonUnmarshalledVehicle.BlockHeater == nil { + t.Fatal("was expecting a non nil Block Heater ptr") + } + if e, a := originalVehicle.BlockHeater.Watts, jsonUnmarshalledVehicle.BlockHeater.Watts; e != a { + t.Fatalf("was expecting watts to be %v, got %v", e, a) + } + + // Serialize as JSONAPI + jsonAPIVehicle := new(bytes.Buffer) + if err = MarshalPayload(jsonAPIVehicle, originalVehicle); err != nil { + t.Fatal(err) + } + + jsonAPIUnmarshalledVehicle := &Vehicle{} + if err = UnmarshalPayload(jsonAPIVehicle, jsonAPIUnmarshalledVehicle); err != nil { + t.Fatal(err) + } + + if jsonAPIUnmarshalledVehicle.BlockHeater == nil { + t.Fatal("was expecting a non nil Block Heater ptr") + } + if e, a := originalVehicle.BlockHeater.Watts, jsonAPIUnmarshalledVehicle.BlockHeater.Watts; e != a { + t.Fatalf("was expecting watts to be %v, got %v", e, a) + } +} + +func TestEmbededStructs_nilStructPtr(t *testing.T) { + originalVehicle := &Vehicle{ + Make: "VW", + Model: "R32", + Year: 2008, + Engine: Engine{ + NumberOfCylinders: 6, + HorsePower: 250, + }, + } + + // Serialize as JSON + jsonVehicle, err := json.Marshal(originalVehicle) + if err != nil { + t.Fatal(err) + } + + jsonUnmarshalledVehicle := &Vehicle{} + json.Unmarshal(jsonVehicle, jsonUnmarshalledVehicle) + + // Proves that the JSON standard lib will NOT allocate a BlockHeater + if e, a := originalVehicle.BlockHeater, jsonUnmarshalledVehicle.BlockHeater; e != a { + t.Fatalf("was expecting BlockHeater to be %v, got %v", e, a) + } + + // Serialize as JSONAPI + jsonAPIVehicle := new(bytes.Buffer) + if err = MarshalPayload(jsonAPIVehicle, originalVehicle); err != nil { + t.Fatal(err) + } + + jsonAPIUnmarshalledVehicle := &Vehicle{} + if err = UnmarshalPayload(jsonAPIVehicle, jsonAPIUnmarshalledVehicle); err != nil { + t.Fatal(err) + } + + if e, a := originalVehicle.BlockHeater, jsonAPIUnmarshalledVehicle.BlockHeater; e != a { + t.Fatalf("was expecting BlockHeater to be %v, got %v", e, a) + } +} + func samplePayloadWithoutIncluded() map[string]interface{} { return map[string]interface{}{ "data": map[string]interface{}{ From 003d45f589e7cc0b7562b4cfd7a2737d5bfbab4b Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Thu, 27 Jul 2017 17:42:37 -0700 Subject: [PATCH 03/13] Added another failing test; this time for the marshalling of duplicate primary annotation fields. --- response_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/response_test.go b/response_test.go index 8c96cfb..f3a0d92 100644 --- a/response_test.go +++ b/response_test.go @@ -1441,6 +1441,47 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { } } +func TestMarshal_duplicatePrimaryAnnotationFromEmbededStructs(t *testing.T) { + type Outer struct { + ID string `jsonapi:"primary,outer"` + Comment + *Post + } + + o := Outer{ + ID: "outer", + Comment: Comment{ID: 1}, + Post: &Post{ID: 5}, + } + var payloadData map[string]interface{} + + // Test the standard libraries JSON handling of dup (ID) fields + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + if e, a := o.ID, payloadData["ID"]; e != a { + t.Fatalf("Was expecting ID to be %v, got %v", e, a) + } + + // Test the JSONAPI lib handling of dup (ID) fields + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + id := data["id"].(string) + if e, a := o.ID, id; e != a { + t.Fatalf("Was expecting ID to be %v, got %v", e, a) + } +} + func testBlog() *Blog { return &Blog{ ID: 5, From 5be05083d821cf687857c36cf7237fe9a4168a10 Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Thu, 27 Jul 2017 17:49:25 -0700 Subject: [PATCH 04/13] Test against 1.9 --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 5786b14..33b4524 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,5 +2,6 @@ language: go go: - 1.7.x - 1.8.x + - 1.9.x - tip script: script/test -v From 662431e860691395a594fb558111b9ac3f5a8fa0 Mon Sep 17 00:00:00 2001 From: skimata Date: Tue, 1 Aug 2017 11:48:27 -0700 Subject: [PATCH 05/13] Feature/embeded structs fix tests (#105) * fix TestEmbededStructs_nonNilStructPtr; bug on loop w/ (multiple) embedded structs; should just continue on success * fix TestMarshal_duplicatePrimaryAnnotationFromEmbeddedStructs; fix order of processing of visitModelNode --- request.go | 8 +++++--- response.go | 25 ++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 6 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 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 cf9619af157c3e5ac96046d0fb65b41c2a2a3dd1 Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Tue, 1 Aug 2017 17:42:38 -0700 Subject: [PATCH 06/13] Moved all embeded struct tests into its own test file; added 3 more failing tests that are related to the handling of duplicate embedded struct fields. --- embeded_structs_test.go | 885 ++++++++++++++++++++++++++++++++++++++++ helpers_test.go | 88 ++++ response_test.go | 746 --------------------------------- 3 files changed, 973 insertions(+), 746 deletions(-) create mode 100644 embeded_structs_test.go create mode 100644 helpers_test.go diff --git a/embeded_structs_test.go b/embeded_structs_test.go new file mode 100644 index 0000000..b9c29c0 --- /dev/null +++ b/embeded_structs_test.go @@ -0,0 +1,885 @@ +package jsonapi + +import ( + "bytes" + "encoding/json" + "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) + } + } +} + +// 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"` + 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"` + } + + 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 + } + + 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", + }, + }, + }) + } + + { + 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 + } + + 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", + }, + }, + }) + } + + { + 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, + }, + }) + } + { + 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) + + // 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", buf.Bytes(), payload) + } + + // 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) + } + } +} + +func TestMarshal_duplicatePrimaryAnnotationFromEmbededStructs(t *testing.T) { + type Outer struct { + ID string `jsonapi:"primary,outer"` + Comment + *Post + } + + o := Outer{ + ID: "outer", + Comment: Comment{ID: 1}, + Post: &Post{ID: 5}, + } + var payloadData map[string]interface{} + + // Test the standard libraries JSON handling of dup (ID) fields + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + if e, a := o.ID, payloadData["ID"]; e != a { + t.Fatalf("Was expecting ID to be %v, got %v", e, a) + } + + // Test the JSONAPI lib handling of dup (ID) fields + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + id := data["id"].(string) + if e, a := o.ID, id; e != a { + t.Fatalf("Was expecting ID to be %v, got %v", e, a) + } +} + +func TestMarshal_duplicateAttributeAnnotationFromEmbededStructs(t *testing.T) { + type Foo struct { + Count uint `json:"count" jsonapi:"attr,count"` + } + type Bar struct { + Count uint `json:"count" jsonapi:"attr,count"` + } + type Outer struct { + ID uint `json:"id" jsonapi:"primary,outer"` + Foo + Bar + } + o := Outer{ + ID: 1, + Foo: Foo{Count: 1}, + Bar: Bar{Count: 2}, + } + + var payloadData map[string]interface{} + + // The standard JSON lib will not serialize either embeded struct's fields if + // a duplicate is encountered + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + if _, found := payloadData["count"]; found { + t.Fatalf("Was not expecting to find the `count` key in the JSON") + } + + // Test the JSONAPI lib handling of dup (attr) fields + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + if _, found := data["attributes"]; found { + t.Fatal("Was not expecting to find any `attributes` in the JSON API") + } +} + +func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsPtrs(t *testing.T) { + type Foo struct { + Count uint `json:"count" jsonapi:"attr,count"` + } + type Bar struct { + Count uint `json:"count" jsonapi:"attr,count"` + } + type Outer struct { + ID uint `json:"id" jsonapi:"primary,outer"` + *Foo + *Bar + } + o := Outer{ + ID: 1, + Foo: &Foo{Count: 1}, + Bar: &Bar{Count: 2}, + } + + var payloadData map[string]interface{} + + // The standard JSON lib will not serialize either embeded struct's fields if + // a duplicate is encountered + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + if _, found := payloadData["count"]; found { + t.Fatalf("Was not expecting to find the `count` key in the JSON") + } + + // Test the JSONAPI lib handling of dup (attr) fields + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + if _, found := data["attributes"]; found { + t.Fatal("Was not expecting to find any `attributes` in the JSON API") + } +} + +func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsMixed(t *testing.T) { + type Foo struct { + Count uint `json:"count" jsonapi:"attr,count"` + } + type Bar struct { + Count uint `json:"count" jsonapi:"attr,count"` + } + type Outer struct { + ID uint `json:"id" jsonapi:"primary,outer"` + *Foo + Bar + } + o := Outer{ + ID: 1, + Foo: &Foo{Count: 1}, + Bar: Bar{Count: 2}, + } + + var payloadData map[string]interface{} + + // The standard JSON lib will not serialize either embeded struct's fields if + // a duplicate is encountered + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + if _, found := payloadData["count"]; found { + t.Fatalf("Was not expecting to find the `count` key in the JSON") + } + + // Test the JSONAPI lib handling of dup (attr) fields; it should serialize + // neither + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + if _, found := data["attributes"]; found { + t.Fatal("Was not expecting to find any `attributes` in the JSON API") + } +} + +func TestMarshal_duplicateFieldFromEmbededStructs_serializationNameDiffers(t *testing.T) { + type Foo struct { + Count uint `json:"foo-count" jsonapi:"attr,foo-count"` + } + type Bar struct { + Count uint `json:"bar-count" jsonapi:"attr,bar-count"` + } + type Outer struct { + ID uint `json:"id" jsonapi:"primary,outer"` + Foo + Bar + } + o := Outer{ + ID: 1, + Foo: Foo{Count: 1}, + Bar: Bar{Count: 2}, + } + + var payloadData map[string]interface{} + + // The standard JSON lib will both the fields since their annotation name + // differs + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + fooJSON, fooFound := payloadData["foo-count"] + if !fooFound { + t.Fatal("Was expecting to find the `foo-count` key in the JSON") + } + if e, a := o.Foo.Count, fooJSON.(float64); e != uint(a) { + t.Fatalf("Was expecting the `foo-count` value to be %v, got %v", e, a) + } + barJSON, barFound := payloadData["bar-count"] + if !barFound { + t.Fatal("Was expecting to find the `bar-count` key in the JSON") + } + if e, a := o.Bar.Count, barJSON.(float64); e != uint(a) { + t.Fatalf("Was expecting the `bar-count` value to be %v, got %v", e, a) + } + + // Test the JSONAPI lib handling; it should serialize both + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + attributes := data["attributes"].(map[string]interface{}) + fooJSONAPI, fooFound := attributes["foo-count"] + if !fooFound { + t.Fatal("Was expecting to find the `foo-count` attribute in the JSON API") + } + if e, a := o.Foo.Count, fooJSONAPI.(float64); e != uint(e) { + t.Fatalf("Was expecting the `foo-count` attrobute to be %v, got %v", e, a) + } + barJSONAPI, barFound := attributes["bar-count"] + if !barFound { + t.Fatal("Was expecting to find the `bar-count` attribute in the JSON API") + } + if e, a := o.Bar.Count, barJSONAPI.(float64); e != uint(e) { + t.Fatalf("Was expecting the `bar-count` attrobute to be %v, got %v", e, a) + } +} diff --git a/helpers_test.go b/helpers_test.go new file mode 100644 index 0000000..6dfdd8f --- /dev/null +++ b/helpers_test.go @@ -0,0 +1,88 @@ +package jsonapi + +import ( + "encoding/json" + "reflect" + "time" +) + +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 +} + +func testBlog() *Blog { + return &Blog{ + ID: 5, + Title: "Title 1", + CreatedAt: time.Now(), + Posts: []*Post{ + &Post{ + ID: 1, + Title: "Foo", + Body: "Bar", + Comments: []*Comment{ + &Comment{ + ID: 1, + Body: "foo", + }, + &Comment{ + ID: 2, + Body: "bar", + }, + }, + LatestComment: &Comment{ + ID: 1, + Body: "foo", + }, + }, + &Post{ + ID: 2, + Title: "Fuubar", + Body: "Bas", + Comments: []*Comment{ + &Comment{ + ID: 1, + Body: "foo", + }, + &Comment{ + ID: 3, + Body: "bas", + }, + }, + LatestComment: &Comment{ + ID: 1, + Body: "foo", + }, + }, + }, + CurrentPost: &Post{ + ID: 1, + Title: "Foo", + Body: "Bar", + Comments: []*Comment{ + &Comment{ + ID: 1, + Body: "foo", + }, + &Comment{ + ID: 2, + Body: "bar", + }, + }, + LatestComment: &Comment{ + ID: 1, + Body: "foo", + }, + }, + } +} diff --git a/response_test.go b/response_test.go index f3a0d92..ab15e4c 100644 --- a/response_test.go +++ b/response_test.go @@ -816,749 +816,3 @@ func TestMarshal_InvalidIntefaceArgument(t *testing.T) { t.Fatal("Was expecting an error") } } - -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) - } - } -} - -// 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"` - 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"` - } - - 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 - } - - 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", - }, - }, - }) - } - - { - 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 - } - - 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", - }, - }, - }) - } - - { - 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, - }, - }) - } - { - 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) - - // 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", buf.Bytes(), payload) - } - - // 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) - } - } -} - -func TestMarshal_duplicatePrimaryAnnotationFromEmbededStructs(t *testing.T) { - type Outer struct { - ID string `jsonapi:"primary,outer"` - Comment - *Post - } - - o := Outer{ - ID: "outer", - Comment: Comment{ID: 1}, - Post: &Post{ID: 5}, - } - var payloadData map[string]interface{} - - // Test the standard libraries JSON handling of dup (ID) fields - jsonData, err := json.Marshal(o) - if err != nil { - t.Fatal(err) - } - if err := json.Unmarshal(jsonData, &payloadData); err != nil { - t.Fatal(err) - } - if e, a := o.ID, payloadData["ID"]; e != a { - t.Fatalf("Was expecting ID to be %v, got %v", e, a) - } - - // Test the JSONAPI lib handling of dup (ID) fields - jsonAPIData := new(bytes.Buffer) - if err := MarshalPayload(jsonAPIData, &o); err != nil { - t.Fatal(err) - } - if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { - t.Fatal(err) - } - data := payloadData["data"].(map[string]interface{}) - id := data["id"].(string) - if e, a := o.ID, id; e != a { - t.Fatalf("Was expecting ID to be %v, got %v", e, a) - } -} - -func testBlog() *Blog { - return &Blog{ - ID: 5, - Title: "Title 1", - CreatedAt: time.Now(), - Posts: []*Post{ - &Post{ - ID: 1, - Title: "Foo", - Body: "Bar", - Comments: []*Comment{ - &Comment{ - ID: 1, - Body: "foo", - }, - &Comment{ - ID: 2, - Body: "bar", - }, - }, - LatestComment: &Comment{ - ID: 1, - Body: "foo", - }, - }, - &Post{ - ID: 2, - Title: "Fuubar", - Body: "Bas", - Comments: []*Comment{ - &Comment{ - ID: 1, - Body: "foo", - }, - &Comment{ - ID: 3, - Body: "bas", - }, - }, - LatestComment: &Comment{ - ID: 1, - Body: "foo", - }, - }, - }, - CurrentPost: &Post{ - ID: 1, - Title: "Foo", - Body: "Bar", - Comments: []*Comment{ - &Comment{ - ID: 1, - Body: "foo", - }, - &Comment{ - ID: 2, - Body: "bar", - }, - }, - LatestComment: &Comment{ - ID: 1, - Body: "foo", - }, - }, - } -} - -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 d94776e6ccbfcdd924f72ab19e9f53e0660c5ca3 Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Tue, 1 Aug 2017 17:44:50 -0700 Subject: [PATCH 07/13] Addressing warnings. --- embeded_structs_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/embeded_structs_test.go b/embeded_structs_test.go index b9c29c0..171cd60 100644 --- a/embeded_structs_test.go +++ b/embeded_structs_test.go @@ -190,10 +190,13 @@ func TestEmbeddedUnmarshalOrder(t *testing.T) { // marshal f (Foo) using json marshaler jsonData, err := json.Marshal(f) + if err != nil { + t.Fatal(err) + } // 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 { + if err = json.Unmarshal(jsonAPIData.Bytes(), &jsonAPIVal); err != nil { t.Fatal(err) } if err = json.Unmarshal(jsonData, &jsonVal); err != nil { From a92a3b75d9ef2872be32ebe13df4936d9b03794f Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Tue, 1 Aug 2017 17:45:33 -0700 Subject: [PATCH 08/13] Fixing more warnings in tests. --- embeded_structs_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/embeded_structs_test.go b/embeded_structs_test.go index 171cd60..659a0e2 100644 --- a/embeded_structs_test.go +++ b/embeded_structs_test.go @@ -256,13 +256,13 @@ func TestEmbeddedMarshalOrder(t *testing.T) { // unmarshal payloadWithInt to f (Foo) using jsonapi unmarshaler; expecting an error f := &Foo{} - if err := UnmarshalPayload(bytes.NewReader(payloadWithInt), f); err == nil { + 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 { + if err = UnmarshalPayload(bytes.NewReader(payloadWithString), f); err != nil { t.Error(err) } if f.Name != "foo" { From cab68dab0ebabb8328c42dba5f9fa6f140bc76f5 Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Tue, 1 Aug 2017 17:50:13 -0700 Subject: [PATCH 09/13] Misc cleanup. --- embeded_structs_test.go | 53 ++++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/embeded_structs_test.go b/embeded_structs_test.go index 659a0e2..59f56d2 100644 --- a/embeded_structs_test.go +++ b/embeded_structs_test.go @@ -73,7 +73,9 @@ func TestIsEmbeddedStruct(t *testing.T) { 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) + t.Errorf( + "Scenario -> %s\nGot -> %v\nExpected -> %v\n", + test.scenario, res, test.expectedRes) } } } @@ -106,7 +108,9 @@ func TestShouldIgnoreField(t *testing.T) { 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) + t.Errorf( + "Scenario -> %s\nGot -> %v\nExpected -> %v\n", + test.scenario, res, test.expectedRes) } } } @@ -136,8 +140,12 @@ func TestIsValidEmbeddedStruct(t *testing.T) { expectedRes: true, }, test{ - scenario: "opt-out", - input: reflect.StructField{Anonymous: true, Tag: "jsonapi:\"-\"", Type: structType}, + scenario: "opt-out", + input: reflect.StructField{ + Anonymous: true, + Tag: "jsonapi:\"-\"", + Type: structType, + }, expectedRes: false, }, test{ @@ -153,16 +161,20 @@ func TestIsValidEmbeddedStruct(t *testing.T) { } for _, test := range tests { - res := (isEmbeddedStruct(test.input) && !shouldIgnoreField(test.input.Tag.Get(annotationJSONAPI))) + 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) + t.Errorf( + "Scenario -> %s\nGot -> %v\nExpected -> %v\n", + test.scenario, res, test.expectedRes) } } } -// 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 +// 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"` @@ -194,7 +206,8 @@ func TestEmbeddedUnmarshalOrder(t *testing.T) { t.Fatal(err) } - // convert bytes to map[string]interface{} so that we can do a semantic JSON comparison + // 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) @@ -204,7 +217,14 @@ func TestEmbeddedUnmarshalOrder(t *testing.T) { } // get to the jsonapi attribute map - jAttrMap := jsonAPIVal["data"].(map[string]interface{})["attributes"].(map[string]interface{}) + jDataMap, ok := jsonAPIVal["data"].(map[string]interface{}) + if !ok { + t.Fatal("Could not parse `data`") + } + jAttrMap, ok := jDataMap["attributes"].(map[string]interface{}) + if !ok { + t.Fatal("Could not parse `attributes`") + } // compare if !reflect.DeepEqual(jAttrMap["Name"], jsonVal["Name"]) { @@ -212,9 +232,10 @@ func TestEmbeddedUnmarshalOrder(t *testing.T) { } } -// 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 +// 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"` @@ -886,3 +907,7 @@ func TestMarshal_duplicateFieldFromEmbededStructs_serializationNameDiffers(t *te t.Fatalf("Was expecting the `bar-count` attrobute to be %v, got %v", e, a) } } + +// TODO: test permutation with outer and embeded attrs + +// TODO: test permutation of relations with embeded structs From 8e4b2619cf2aef10a7602a4f6f83391c0a1a11a8 Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Tue, 1 Aug 2017 18:01:13 -0700 Subject: [PATCH 10/13] Added two additional tests (currently passing) that test the cases where an Outer struct had an attr field and an embedded field provided a field with the same attr name. --- embeded_structs_test.go | 91 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-) diff --git a/embeded_structs_test.go b/embeded_structs_test.go index 59f56d2..9399fb6 100644 --- a/embeded_structs_test.go +++ b/embeded_structs_test.go @@ -669,7 +669,8 @@ func TestMarshal_duplicatePrimaryAnnotationFromEmbededStructs(t *testing.T) { } var payloadData map[string]interface{} - // Test the standard libraries JSON handling of dup (ID) fields + // Test the standard libraries JSON handling of dup (ID) fields - it uses + // the Outer's ID jsonData, err := json.Marshal(o) if err != nil { t.Fatal(err) @@ -908,6 +909,92 @@ func TestMarshal_duplicateFieldFromEmbededStructs_serializationNameDiffers(t *te } } -// TODO: test permutation with outer and embeded attrs +func TestMarshal_embededStruct_providesDuplicateAttr(t *testing.T) { + type Foo struct { + Number uint `json:"count" jsonapi:"attr,count"` + } + type Outer struct { + Foo + ID uint `json:"id" jsonapi:"primary,outer"` + Count uint `json:"count" jsonapi:"attr,count"` + } + o := Outer{ + ID: 1, + Count: 1, + Foo: Foo{Number: 5}, + } + var payloadData map[string]interface{} + + // The standard JSON lib will take the count annotated field from the Outer + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + if e, a := o.Count, payloadData["count"].(float64); e != uint(a) { + t.Fatalf("Was expecting a JSON `count` of %v, got %v", e, a) + } + + // In JSON API the handling should be that the Outer annotated count field is + // serialized into `attributes` + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + attributes := data["attributes"].(map[string]interface{}) + if e, a := o.Count, attributes["count"].(float64); e != uint(a) { + t.Fatalf("Was expecting a JSON API `count` attribute of %v, got %v", e, a) + } +} + +func TestMarshal_embededStructPtr_providesDuplicateAttr(t *testing.T) { + type Foo struct { + Number uint `json:"count" jsonapi:"attr,count"` + } + type Outer struct { + *Foo + ID uint `json:"id" jsonapi:"primary,outer"` + Count uint `json:"count" jsonapi:"attr,count"` + } + o := Outer{ + ID: 1, + Count: 1, + Foo: &Foo{Number: 5}, + } + var payloadData map[string]interface{} + + // The standard JSON lib will take the count annotated field from the Outer + jsonData, err := json.Marshal(o) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonData, &payloadData); err != nil { + t.Fatal(err) + } + if e, a := o.Count, payloadData["count"].(float64); e != uint(a) { + t.Fatalf("Was expecting a JSON `count` of %v, got %v", e, a) + } + + // In JSON API the handling should be that the Outer annotated count field is + // serialized into `attributes` + jsonAPIData := new(bytes.Buffer) + if err := MarshalPayload(jsonAPIData, &o); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(jsonAPIData.Bytes(), &payloadData); err != nil { + t.Fatal(err) + } + data := payloadData["data"].(map[string]interface{}) + attributes := data["attributes"].(map[string]interface{}) + if e, a := o.Count, attributes["count"].(float64); e != uint(a) { + t.Fatalf("Was expecting a JSON API `count` attribute of %v, got %v", e, a) + } +} // TODO: test permutation of relations with embeded structs From 0ec2f4e6b9b964638321c96799601b4a3ce35f3b Mon Sep 17 00:00:00 2001 From: Aren Patel Date: Tue, 1 Aug 2017 18:03:13 -0700 Subject: [PATCH 11/13] Fixed embedded spelling mistakes. --- embeded_structs_test.go | 22 +++++++++++----------- models_test.go | 2 +- request_test.go | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/embeded_structs_test.go b/embeded_structs_test.go index 9399fb6..17381ca 100644 --- a/embeded_structs_test.go +++ b/embeded_structs_test.go @@ -655,7 +655,7 @@ func TestMarshalUnmarshalCompositeStruct(t *testing.T) { } } -func TestMarshal_duplicatePrimaryAnnotationFromEmbededStructs(t *testing.T) { +func TestMarshal_duplicatePrimaryAnnotationFromEmbeddedStructs(t *testing.T) { type Outer struct { ID string `jsonapi:"primary,outer"` Comment @@ -697,7 +697,7 @@ func TestMarshal_duplicatePrimaryAnnotationFromEmbededStructs(t *testing.T) { } } -func TestMarshal_duplicateAttributeAnnotationFromEmbededStructs(t *testing.T) { +func TestMarshal_duplicateAttributeAnnotationFromEmbeddedStructs(t *testing.T) { type Foo struct { Count uint `json:"count" jsonapi:"attr,count"` } @@ -717,7 +717,7 @@ func TestMarshal_duplicateAttributeAnnotationFromEmbededStructs(t *testing.T) { var payloadData map[string]interface{} - // The standard JSON lib will not serialize either embeded struct's fields if + // The standard JSON lib will not serialize either embedded struct's fields if // a duplicate is encountered jsonData, err := json.Marshal(o) if err != nil { @@ -744,7 +744,7 @@ func TestMarshal_duplicateAttributeAnnotationFromEmbededStructs(t *testing.T) { } } -func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsPtrs(t *testing.T) { +func TestMarshal_duplicateAttributeAnnotationFromEmbeddedStructsPtrs(t *testing.T) { type Foo struct { Count uint `json:"count" jsonapi:"attr,count"` } @@ -764,7 +764,7 @@ func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsPtrs(t *testing.T var payloadData map[string]interface{} - // The standard JSON lib will not serialize either embeded struct's fields if + // The standard JSON lib will not serialize either embedded struct's fields if // a duplicate is encountered jsonData, err := json.Marshal(o) if err != nil { @@ -791,7 +791,7 @@ func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsPtrs(t *testing.T } } -func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsMixed(t *testing.T) { +func TestMarshal_duplicateAttributeAnnotationFromEmbeddedStructsMixed(t *testing.T) { type Foo struct { Count uint `json:"count" jsonapi:"attr,count"` } @@ -811,7 +811,7 @@ func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsMixed(t *testing. var payloadData map[string]interface{} - // The standard JSON lib will not serialize either embeded struct's fields if + // The standard JSON lib will not serialize either embedded struct's fields if // a duplicate is encountered jsonData, err := json.Marshal(o) if err != nil { @@ -839,7 +839,7 @@ func TestMarshal_duplicateAttributeAnnotationFromEmbededStructsMixed(t *testing. } } -func TestMarshal_duplicateFieldFromEmbededStructs_serializationNameDiffers(t *testing.T) { +func TestMarshal_duplicateFieldFromEmbeddedStructs_serializationNameDiffers(t *testing.T) { type Foo struct { Count uint `json:"foo-count" jsonapi:"attr,foo-count"` } @@ -909,7 +909,7 @@ func TestMarshal_duplicateFieldFromEmbededStructs_serializationNameDiffers(t *te } } -func TestMarshal_embededStruct_providesDuplicateAttr(t *testing.T) { +func TestMarshal_embeddedStruct_providesDuplicateAttr(t *testing.T) { type Foo struct { Number uint `json:"count" jsonapi:"attr,count"` } @@ -953,7 +953,7 @@ func TestMarshal_embededStruct_providesDuplicateAttr(t *testing.T) { } } -func TestMarshal_embededStructPtr_providesDuplicateAttr(t *testing.T) { +func TestMarshal_embeddedStructPtr_providesDuplicateAttr(t *testing.T) { type Foo struct { Number uint `json:"count" jsonapi:"attr,count"` } @@ -997,4 +997,4 @@ func TestMarshal_embededStructPtr_providesDuplicateAttr(t *testing.T) { } } -// TODO: test permutation of relations with embeded structs +// TODO: test permutation of relations with embedded structs diff --git a/models_test.go b/models_test.go index 1059ad4..925189e 100644 --- a/models_test.go +++ b/models_test.go @@ -156,7 +156,7 @@ func (bc *BadComment) JSONAPILinks() *Links { } } -// Embeded Struct Models +// Embedded Struct Models type Engine struct { NumberOfCylinders uint `jsonapi:"attr,cylinders"` HorsePower uint `jsonapi:"attr,hp"` diff --git a/request_test.go b/request_test.go index ea9a15f..13172c8 100644 --- a/request_test.go +++ b/request_test.go @@ -703,7 +703,7 @@ func TestManyPayload_withLinks(t *testing.T) { } } -func TestEmbededStructs_nonNilStructPtr(t *testing.T) { +func TestEmbeddedStructs_nonNilStructPtr(t *testing.T) { originalVehicle := &Vehicle{ Make: "VW", Model: "R32", @@ -753,7 +753,7 @@ func TestEmbededStructs_nonNilStructPtr(t *testing.T) { } } -func TestEmbededStructs_nilStructPtr(t *testing.T) { +func TestEmbeddedStructs_nilStructPtr(t *testing.T) { originalVehicle := &Vehicle{ Make: "VW", Model: "R32", From 66e21ca95323458701e8fd824b352e32d4fea2bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Mar=C3=ADa=20Signanini?= Date: Tue, 1 May 2018 14:28:09 -0400 Subject: [PATCH 12/13] Implement error object links. --- errors.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/errors.go b/errors.go index ed7fa9f..9696e86 100644 --- a/errors.go +++ b/errors.go @@ -23,6 +23,14 @@ type ErrorsPayload struct { Errors []*ErrorObject `json:"errors"` } +// ErrorObjectLinks is an implementation of the JSON API error links payload. +// +// For more information on the JSON API spec's error links objects, see: http://jsonapi.org/format/#error-objects +type ErrorObjectLinks struct { + // About is a link that leads to further details about this particular occurrence of the problem. + About string `json:"about"` +} + // ErrorObject is an `Error` implementation as well as an implementation of the JSON API error object. // // The main idea behind this struct is that you can use it directly in your code as an error type @@ -45,6 +53,9 @@ type ErrorObject struct { // Code is an application-specific error code, expressed as a string value. Code string `json:"code,omitempty"` + // Links is an implementation of the JSON API error links payload. + Links *ErrorObjectLinks `json:"links,omitempty"` + // Meta is an object containing non-standard meta-information about the error. Meta *map[string]interface{} `json:"meta,omitempty"` } From 466da34397d87af2bbb6615c03d0ea8b138082b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Mar=C3=ADa=20Signanini?= Date: Tue, 1 May 2018 14:28:09 -0400 Subject: [PATCH 13/13] Implement error object links. --- errors.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/errors.go b/errors.go index ed7fa9f..9696e86 100644 --- a/errors.go +++ b/errors.go @@ -23,6 +23,14 @@ type ErrorsPayload struct { Errors []*ErrorObject `json:"errors"` } +// ErrorObjectLinks is an implementation of the JSON API error links payload. +// +// For more information on the JSON API spec's error links objects, see: http://jsonapi.org/format/#error-objects +type ErrorObjectLinks struct { + // About is a link that leads to further details about this particular occurrence of the problem. + About string `json:"about"` +} + // ErrorObject is an `Error` implementation as well as an implementation of the JSON API error object. // // The main idea behind this struct is that you can use it directly in your code as an error type @@ -45,6 +53,9 @@ type ErrorObject struct { // Code is an application-specific error code, expressed as a string value. Code string `json:"code,omitempty"` + // Links is an implementation of the JSON API error links payload. + Links *ErrorObjectLinks `json:"links,omitempty"` + // Meta is an object containing non-standard meta-information about the error. Meta *map[string]interface{} `json:"meta,omitempty"` }