-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Embeded structs fix tests part2 #108
base: feature/embeded-structs
Are you sure you want to change the base?
Changes from all commits
fb16f7c
92996bf
ca31c07
d00c64c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,13 +38,71 @@ type Node struct { | |
Type string `json:"type"` | ||
ID string `json:"id,omitempty"` | ||
ClientID string `json:"client-id,omitempty"` | ||
Attributes map[string]interface{} `json:"attributes,omitempty"` | ||
Attributes attributes `json:"attributes,omitempty"` | ||
Relationships map[string]interface{} `json:"relationships,omitempty"` | ||
Links *Links `json:"links,omitempty"` | ||
Meta *Meta `json:"meta,omitempty"` | ||
} | ||
|
||
func (n *Node) handleNodeErrors() { | ||
for k, v := range n.Attributes { | ||
if _, ok := v.(nodeError); ok { | ||
delete(n.Attributes, k) | ||
} | ||
} | ||
} | ||
|
||
type attributes map[string]interface{} | ||
|
||
// this attributes setter | ||
// * adds new entries as-is | ||
// * tracks dominant field conflicts | ||
func (a attributes) set(k string, v interface{}) { | ||
if val, ok := a[k]; ok { | ||
if ne, ok := val.(nodeError); ok { | ||
ne.Add(k, v) | ||
return | ||
} | ||
a[k] = newDominantFieldConflict(k, val, v) | ||
} else { | ||
a[k] = v | ||
} | ||
} | ||
|
||
// mergeAttributes consolidates 2 attribute maps | ||
// if there are conflicting keys, the values in the argument take priority | ||
func (n *Node) mergeAttributes(attrs attributes) { | ||
for k, v := range attrs { | ||
n.Attributes[k] = v | ||
} | ||
} | ||
|
||
// mergeAttributes consolidates multiple nodes into a single consolidated node | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment doesn't seem to make sense here: seems to be for a different function. |
||
// the last node value has a higher priority over others in setting single value types (i.e. node.ID, node.Type) | ||
// if there are conflicting attributes, those will get flagged as errors | ||
func combinePeerNodes(nodes []*Node) *Node { | ||
n := &Node{} | ||
for _, node := range nodes { | ||
n.peerMerge(node) | ||
} | ||
return n | ||
} | ||
|
||
// merge - a simple merge where the values in the argument have a higher priority | ||
func (n *Node) merge(node *Node) { | ||
n.mergeFunc(node, n.mergeAttributes) | ||
} | ||
|
||
// peerMerge - when merging peers, we need to track nodeErrors | ||
func (n *Node) peerMerge(node *Node) { | ||
n.mergeFunc(node, func(attrs attributes) { | ||
for k, v := range node.Attributes { | ||
n.Attributes.set(k, v) | ||
} | ||
}) | ||
} | ||
|
||
func (n *Node) mergeFunc(node *Node, attrSetter func(attrs attributes)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move |
||
if node.Type != "" { | ||
n.Type = node.Type | ||
} | ||
|
@@ -60,9 +118,7 @@ func (n *Node) merge(node *Node) { | |
if n.Attributes == nil && node.Attributes != nil { | ||
n.Attributes = make(map[string]interface{}) | ||
} | ||
for k, v := range node.Attributes { | ||
n.Attributes[k] = v | ||
} | ||
attrSetter(node.Attributes) | ||
|
||
if n.Relationships == nil && node.Relationships != nil { | ||
n.Relationships = make(map[string]interface{}) | ||
|
@@ -183,3 +239,36 @@ func deepCopyNode(n *Node) *Node { | |
} | ||
return © | ||
} | ||
|
||
// nodeError is used to track errors in processing Node related values | ||
type nodeError interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there more than one of these things? Why do we need an interface? |
||
Error() string | ||
Add(key string, val interface{}) | ||
} | ||
|
||
// dominantFieldConflict is a specific type of marshaling scenario that the standard json lib treats as an error | ||
// comment from json.dominantField(): "If there are multiple top-level fields...This condition is an error in Go and we skip" | ||
// tracking the key and conflicting values for future use to possibly surface an error | ||
type dominantFieldConflict struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename |
||
key string | ||
vals []interface{} | ||
} | ||
|
||
func newDominantFieldConflict(key string, vals ...interface{}) interface{} { | ||
return &dominantFieldConflict{ | ||
key: key, | ||
vals: vals, | ||
} | ||
} | ||
|
||
func (dfc *dominantFieldConflict) Error() string { | ||
return fmt.Sprintf("there is a conflict with this attribute: %s", dfc.key) | ||
} | ||
|
||
func (dfc *dominantFieldConflict) Add(key string, val interface{}) { | ||
dfc.key = key | ||
if dfc.vals == nil { | ||
dfc.vals = make([]interface{}, 0) | ||
} | ||
dfc.vals = append(dfc.vals, val) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,226 @@ | ||
package jsonapi | ||
|
||
import ( | ||
"reflect" | ||
"testing" | ||
) | ||
|
||
func TestHandleNodeErrors(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
hasNodeError bool | ||
input *Node | ||
expected *Node | ||
}{ | ||
{ | ||
name: "has no errors", | ||
hasNodeError: false, | ||
input: &Node{Attributes: attributes{"foo": true, "bar": false}}, | ||
expected: &Node{Attributes: attributes{"foo": true, "bar": false}}, | ||
}, | ||
{ | ||
name: "has an error", | ||
hasNodeError: true, | ||
input: &Node{Attributes: attributes{"foo": true, "bar": &dominantFieldConflict{key: "bar", vals: []interface{}{true, false}}}}, | ||
expected: &Node{Attributes: attributes{"foo": true}}, | ||
}, | ||
{ | ||
name: "has a couple errors", | ||
hasNodeError: true, | ||
input: &Node{ | ||
Attributes: attributes{ | ||
"foo": true, | ||
"bar": &dominantFieldConflict{key: "bar", vals: []interface{}{true, false}}, | ||
"bat": &dominantFieldConflict{key: "bat", vals: []interface{}{true, false}}, | ||
}, | ||
}, | ||
expected: &Node{Attributes: attributes{"foo": true}}, | ||
}, | ||
} | ||
|
||
for _, scenario := range tests { | ||
t.Logf("scenario: %s\n", scenario.name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use t.Run. See "Table-driven tests using subtests" @ https://blog.golang.org/subtests |
||
if scenario.hasNodeError && reflect.DeepEqual(scenario.input, scenario.expected) { | ||
t.Error("expected input and expected to be different") | ||
} | ||
scenario.input.handleNodeErrors() | ||
if !reflect.DeepEqual(scenario.input, scenario.expected) { | ||
t.Errorf("Got\n%#v\nExpected\n%#v\n", scenario.input, scenario.expected) | ||
} | ||
} | ||
} | ||
|
||
func TestSetAttributes(t *testing.T) { | ||
key := "foo" | ||
attr := attributes{} | ||
|
||
// set first val | ||
attr.set(key, false) | ||
|
||
// check presence of first val | ||
val, ok := attr[key] | ||
if !ok { | ||
t.Errorf("expected attributes to have key: %s\n", key) | ||
} | ||
|
||
// assert first val is not an error | ||
_, ok = val.(nodeError) | ||
if ok { | ||
t.Errorf("val stored for key (%s) should NOT be a nodeError\n", key) | ||
} | ||
|
||
// add second val; same key | ||
attr.set(key, true) | ||
|
||
// assert val converted to an error | ||
_, ok = attr[key].(nodeError) | ||
if !ok { | ||
t.Errorf("val stored for key (%s) should be a nodeError\n", key) | ||
} | ||
|
||
// add third val; same key | ||
attr.set(key, nil) | ||
|
||
// assert val remains an error | ||
_, ok = attr[key].(nodeError) | ||
if !ok { | ||
t.Errorf("val stored for key (%s) should be a nodeError\n", key) | ||
} | ||
} | ||
|
||
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 TestCombinePeerNodesNoConflict(t *testing.T) { | ||
brother := &Node{ | ||
Type: "brother", | ||
ID: "99", | ||
ClientID: "9999", | ||
Attributes: map[string]interface{}{"fizz": "buzz"}, | ||
Relationships: map[string]interface{}{"father": "Joe"}, | ||
} | ||
|
||
sister := &Node{ | ||
Type: "sister", | ||
ID: "11", | ||
ClientID: "1111", | ||
Attributes: map[string]interface{}{"timbuk": 2}, | ||
Relationships: map[string]interface{}{"mother": "Mary"}, | ||
} | ||
|
||
expected := &Node{ | ||
Type: "sister", | ||
ID: "11", | ||
ClientID: "1111", | ||
Attributes: map[string]interface{}{"fizz": "buzz", "timbuk": 2}, | ||
Relationships: map[string]interface{}{"father": "Joe", "mother": "Mary"}, | ||
} | ||
|
||
actual := combinePeerNodes([]*Node{brother, sister}) | ||
|
||
if !reflect.DeepEqual(actual, expected) { | ||
t.Errorf("Got %+v Expected %+v", actual, expected) | ||
} | ||
} | ||
func TestCombinePeerNodesWithConflict(t *testing.T) { | ||
brother := &Node{ | ||
Type: "brother", | ||
ID: "99", | ||
ClientID: "9999", | ||
Attributes: map[string]interface{}{"timbuk": 2}, | ||
Relationships: map[string]interface{}{"father": "Joe"}, | ||
} | ||
|
||
sister := &Node{ | ||
Type: "sister", | ||
ID: "11", | ||
ClientID: "1111", | ||
Attributes: map[string]interface{}{"timbuk": 2}, | ||
Relationships: map[string]interface{}{"mother": "Mary"}, | ||
} | ||
|
||
expected := &Node{ | ||
Type: "sister", | ||
ID: "11", | ||
ClientID: "1111", | ||
Attributes: map[string]interface{}{"timbuk": &dominantFieldConflict{key: "timbuk", vals: []interface{}{2, 2}}}, | ||
Relationships: map[string]interface{}{"father": "Joe", "mother": "Mary"}, | ||
} | ||
|
||
actual := combinePeerNodes([]*Node{brother, sister}) | ||
|
||
if !reflect.DeepEqual(actual, expected) { | ||
t.Errorf("Got %+v Expected %+v", actual, expected) | ||
} | ||
} | ||
|
||
func TestDeepCopyNode(t *testing.T) { | ||
source := &Node{ | ||
Type: "thing", | ||
ID: "1", | ||
ClientID: "2", | ||
Attributes: attributes{"key": "val"}, | ||
Relationships: map[string]interface{}{"key": "val"}, | ||
Links: &Links{"key": "val"}, | ||
Meta: &Meta{"key": "val"}, | ||
} | ||
|
||
badCopy := *source | ||
if !reflect.DeepEqual(badCopy, *source) { | ||
t.Errorf("Got %+v Expected %+v", badCopy, *source) | ||
} | ||
if reflect.ValueOf(badCopy.Attributes).Pointer() != reflect.ValueOf(source.Attributes).Pointer() { | ||
t.Error("Expected map address to be the same") | ||
} | ||
if reflect.ValueOf(badCopy.Relationships).Pointer() != reflect.ValueOf(source.Relationships).Pointer() { | ||
t.Error("Expected map address to be the same") | ||
} | ||
if reflect.ValueOf(badCopy.Links).Pointer() != reflect.ValueOf(source.Links).Pointer() { | ||
t.Error("Expected map address to be the same") | ||
} | ||
if reflect.ValueOf(badCopy.Meta).Pointer() != reflect.ValueOf(source.Meta).Pointer() { | ||
t.Error("Expected map address to be the same") | ||
} | ||
|
||
deepCopy := deepCopyNode(source) | ||
if !reflect.DeepEqual(*deepCopy, *source) { | ||
t.Errorf("Got %+v Expected %+v", *deepCopy, *source) | ||
} | ||
if reflect.ValueOf(deepCopy.Attributes).Pointer() == reflect.ValueOf(source.Attributes).Pointer() { | ||
t.Error("Expected map address to be different") | ||
} | ||
if reflect.ValueOf(deepCopy.Relationships).Pointer() == reflect.ValueOf(source.Relationships).Pointer() { | ||
t.Error("Expected map address to be different") | ||
} | ||
if reflect.ValueOf(deepCopy.Links).Pointer() == reflect.ValueOf(source.Links).Pointer() { | ||
t.Error("Expected map address to be different") | ||
} | ||
if reflect.ValueOf(deepCopy.Meta).Pointer() == reflect.ValueOf(source.Meta).Pointer() { | ||
t.Error("Expected map address to be different") | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename
ok
tohasKey
. Convention is to useok
for type assertion only.