Skip to content

Commit 81a76b6

Browse files
brandoncbrandonc
authored
Merge pull request #33 from hashicorp/brandonc/revert_circular_relationship_handling
Revert circular relationship handling
2 parents 876fb53 + 74c1838 commit 81a76b6

File tree

4 files changed

+24
-201
lines changed

4 files changed

+24
-201
lines changed

‎.github/CODEOWNERS‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
# More on CODEOWNERS files: https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners
33

44
# Default owners
5-
* @hashicorp/tf-core-cloud
65
* @hashicorp/team-ip-compliance
6+
* @hashicorp/tf-core-cloud
77

88
# Add override rules below. Each line is a file/folder pattern followed by one or more owners.
99
# Being an owner means those groups or individuals will be added as reviewers to PRs affecting

‎models_test.go‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -90,36 +90,15 @@ type GenericInterface struct {
9090
Data interface{} `jsonapi:"attr,interface"`
9191
}
9292

93-
type Organization struct {
94-
ID int `jsonapi:"primary,organizations"`
95-
ClientID string `jsonapi:"client-id"`
96-
Name string `jsonapi:"attr,title"`
97-
DefaultProject *Project `jsonapi:"relation,default_project"`
98-
CreatedAt time.Time `jsonapi:"attr,created_at"`
99-
100-
Links Links `jsonapi:"links,omitempty"`
101-
}
102-
103-
type Project struct {
104-
ID int `jsonapi:"primary,projects"`
105-
ClientID string `jsonapi:"client-id"`
106-
Name string `jsonapi:"attr,name"`
107-
Organization *Organization `jsonapi:"relation,organization"`
108-
109-
Links Links `jsonapi:"links,omitempty"`
110-
}
111-
11293
type Blog struct {
113-
ID int `jsonapi:"primary,blogs"`
114-
ClientID string `jsonapi:"client-id"`
115-
Title string `jsonapi:"attr,title"`
116-
CurrentPostID int `jsonapi:"attr,current_post_id"`
117-
CreatedAt time.Time `jsonapi:"attr,created_at"`
118-
ViewCount int `jsonapi:"attr,view_count"`
119-
Posts []*Post `jsonapi:"relation,posts"`
120-
CurrentPost *Post `jsonapi:"relation,current_post"`
121-
Organization *Organization `jsonapi:"relation,organization"`
122-
Project *Project `jsonapi:"relation,project"`
94+
ID int `jsonapi:"primary,blogs"`
95+
ClientID string `jsonapi:"client-id"`
96+
Title string `jsonapi:"attr,title"`
97+
Posts []*Post `jsonapi:"relation,posts"`
98+
CurrentPost *Post `jsonapi:"relation,current_post"`
99+
CurrentPostID int `jsonapi:"attr,current_post_id"`
100+
CreatedAt time.Time `jsonapi:"attr,created_at"`
101+
ViewCount int `jsonapi:"attr,view_count"`
123102

124103
Links Links `jsonapi:"links,omitempty"`
125104
}

‎request.go‎

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,6 @@ func newErrUnsupportedPtrType(rf reflect.Value, t reflect.Type, structField refl
6060
return ErrUnsupportedPtrType{rf, t, structField}
6161
}
6262

63-
type includedNode struct {
64-
node *Node
65-
model *reflect.Value
66-
}
67-
6863
// UnmarshalPayload converts an io into a struct instance using jsonapi tags on
6964
// struct fields. This method supports single request payloads only, at the
7065
// moment. Bulk creates and updates are not supported yet.
@@ -99,19 +94,19 @@ type includedNode struct {
9994
// model interface{} should be a pointer to a struct.
10095
func UnmarshalPayload(in io.Reader, model interface{}) error {
10196
payload := new(OnePayload)
102-
included := make(map[string]*includedNode)
10397

10498
if err := json.NewDecoder(in).Decode(payload); err != nil {
10599
return err
106100
}
107101

108102
if payload.Included != nil {
109-
for _, include := range payload.Included {
110-
key := fmt.Sprintf("%s,%s", include.Type, include.ID)
111-
included[key] = &includedNode{include, nil}
103+
includedMap := make(map[string]*Node)
104+
for _, included := range payload.Included {
105+
key := fmt.Sprintf("%s,%s", included.Type, included.ID)
106+
includedMap[key] = included
112107
}
113108

114-
return unmarshalNode(payload.Data, reflect.ValueOf(model), &included)
109+
return unmarshalNode(payload.Data, reflect.ValueOf(model), &includedMap)
115110
}
116111
return unmarshalNode(payload.Data, reflect.ValueOf(model), nil)
117112
}
@@ -125,19 +120,19 @@ func UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) {
125120
return nil, err
126121
}
127122

128-
models := []interface{}{} // will be populated from the "data"
129-
included := map[string]*includedNode{} // will be populate from the "included"
123+
models := []interface{}{} // will be populated from the "data"
124+
includedMap := map[string]*Node{} // will be populate from the "included"
130125

131126
if payload.Included != nil {
132-
for _, include := range payload.Included {
133-
key := fmt.Sprintf("%s,%s", include.Type, include.ID)
134-
included[key] = &includedNode{include, nil}
127+
for _, included := range payload.Included {
128+
key := fmt.Sprintf("%s,%s", included.Type, included.ID)
129+
includedMap[key] = included
135130
}
136131
}
137132

138133
for _, data := range payload.Data {
139134
model := reflect.New(t.Elem())
140-
err := unmarshalNode(data, model, &included)
135+
err := unmarshalNode(data, model, &includedMap)
141136
if err != nil {
142137
return nil, err
143138
}
@@ -268,7 +263,7 @@ func getStructTags(field reflect.StructField) ([]string, error) {
268263

269264
// unmarshalNodeMaybeChoice populates a model that may or may not be
270265
// a choice type struct that corresponds to a polyrelation or relation
271-
func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, choiceTypeMapping map[string]structFieldIndex, included *map[string]*includedNode) error {
266+
func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, choiceTypeMapping map[string]structFieldIndex, included *map[string]*Node) error {
272267
// This will hold either the value of the choice type model or the actual
273268
// model, depending on annotation
274269
var actualModel = *m
@@ -305,7 +300,7 @@ func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, c
305300
return nil
306301
}
307302

308-
func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includedNode) (err error) {
303+
func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) (err error) {
309304
defer func() {
310305
if r := recover(); r != nil {
311306
err = fmt.Errorf("data is not a jsonapi representation of '%v'", model.Type())
@@ -534,23 +529,6 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ
534529
continue
535530
}
536531

537-
// Check if the item in the relationship was already processed elsewhere. Avoids potential infinite recursive loops
538-
// caused by circular references between included relationships (two included items include one another)
539-
includedKey := fmt.Sprintf("%s,%s", relationship.Data.Type, relationship.Data.ID)
540-
if included != nil && (*included)[includedKey] != nil {
541-
if (*included)[includedKey].model != nil {
542-
fieldValue.Set(*(*included)[includedKey].model)
543-
} else {
544-
(*included)[includedKey].model = &m
545-
err := unmarshalNodeMaybeChoice(&m, (*included)[includedKey].node, annotation, choiceMapping, included)
546-
if err != nil {
547-
er = err
548-
break
549-
}
550-
fieldValue.Set(m)
551-
}
552-
continue
553-
}
554532
err = unmarshalNodeMaybeChoice(&m, relationship.Data, annotation, choiceMapping, included)
555533
if err != nil {
556534
er = err
@@ -612,11 +590,11 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ
612590
return er
613591
}
614592

615-
func fullNode(n *Node, included *map[string]*includedNode) *Node {
593+
func fullNode(n *Node, included *map[string]*Node) *Node {
616594
includedKey := fmt.Sprintf("%s,%s", n.Type, n.ID)
617595

618596
if included != nil && (*included)[includedKey] != nil {
619-
return (*included)[includedKey].node
597+
return (*included)[includedKey]
620598
}
621599

622600
return n

‎request_test.go‎

Lines changed: 0 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -811,41 +811,6 @@ func TestUnmarshalRelationships(t *testing.T) {
811811
}
812812
}
813813

814-
func TestUnmarshalMany_relationships_with_circular_inclusion(t *testing.T) {
815-
data := samplePayloadWithCircularInclusion()
816-
payload, err := json.Marshal(data)
817-
if err != nil {
818-
t.Fatal(err)
819-
}
820-
in := bytes.NewReader(payload)
821-
model := reflect.TypeOf(new(Blog))
822-
823-
out, err := UnmarshalManyPayload(in, model)
824-
if err != nil {
825-
t.Fatal(err)
826-
}
827-
828-
result_1 := out[0].(*Blog)
829-
830-
if result_1.Project != result_1.Organization.DefaultProject {
831-
t.Errorf("expected blog.project (%p) to hold the same pointer as blog.organization.default-project (%p) ", result_1.Project, result_1.Organization.DefaultProject)
832-
}
833-
834-
if result_1.Organization != result_1.Project.Organization {
835-
t.Errorf("expected blog.organization (%p) to hold the same pointer as blog.project.organization (%p)", result_1.Organization, result_1.Project.Organization)
836-
}
837-
838-
result_2 := out[1].(*Blog)
839-
840-
if result_2.Project != result_2.Organization.DefaultProject {
841-
t.Errorf("expected blog.project (%p) to hold the same pointer as blog.organization.default-project (%p) ", result_2.Project, result_2.Organization.DefaultProject)
842-
}
843-
844-
if result_2.Organization != result_2.Project.Organization {
845-
t.Errorf("expected blog.organization (%p) to hold the same pointer as blog.project.organization (%p)", result_2.Organization, result_2.Project.Organization)
846-
}
847-
}
848-
849814
func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) {
850815
in := bytes.NewReader([]byte(`{
851816
"data": {
@@ -1535,105 +1500,6 @@ func TestUnmarshalCustomTypeAttributes_ErrInvalidType(t *testing.T) {
15351500
}
15361501
}
15371502

1538-
func samplePayloadWithCircularInclusion() *ManyPayload {
1539-
payload := &ManyPayload{
1540-
Data: []*Node{
1541-
{
1542-
Type: "blogs",
1543-
ClientID: "1",
1544-
ID: "1",
1545-
Attributes: map[string]interface{}{
1546-
"title": "Foo",
1547-
"current_post_id": 1,
1548-
"created_at": 1436216820,
1549-
"view_count": 1000,
1550-
},
1551-
Relationships: map[string]interface{}{
1552-
"project": &RelationshipOneNode{
1553-
Data: &Node{
1554-
Type: "projects",
1555-
ClientID: "1",
1556-
ID: "1",
1557-
},
1558-
},
1559-
"organization": &RelationshipOneNode{
1560-
Data: &Node{
1561-
Type: "organizations",
1562-
ClientID: "1",
1563-
ID: "1",
1564-
},
1565-
},
1566-
},
1567-
},
1568-
{
1569-
Type: "blogs",
1570-
ClientID: "2",
1571-
ID: "2",
1572-
Attributes: map[string]interface{}{
1573-
"title": "Foo2",
1574-
"current_post_id": 1,
1575-
"created_at": 1436216820,
1576-
"view_count": 1000,
1577-
},
1578-
Relationships: map[string]interface{}{
1579-
"project": &RelationshipOneNode{
1580-
Data: &Node{
1581-
Type: "projects",
1582-
ClientID: "1",
1583-
ID: "1",
1584-
},
1585-
},
1586-
"organization": &RelationshipOneNode{
1587-
Data: &Node{
1588-
Type: "organizations",
1589-
ClientID: "1",
1590-
ID: "1",
1591-
},
1592-
},
1593-
},
1594-
},
1595-
},
1596-
Included: []*Node{
1597-
{
1598-
Type: "projects",
1599-
ClientID: "1",
1600-
ID: "1",
1601-
Attributes: map[string]interface{}{
1602-
"name": "Bar",
1603-
},
1604-
Relationships: map[string]interface{}{
1605-
"organization": &RelationshipOneNode{
1606-
Data: &Node{
1607-
Type: "organizations",
1608-
ClientID: "1",
1609-
ID: "1",
1610-
},
1611-
},
1612-
},
1613-
},
1614-
{
1615-
Type: "organizations",
1616-
ClientID: "1",
1617-
ID: "1",
1618-
Attributes: map[string]interface{}{
1619-
"name": "Baz",
1620-
},
1621-
Relationships: map[string]interface{}{
1622-
"default_project": &RelationshipOneNode{
1623-
Data: &Node{
1624-
Type: "projects",
1625-
ClientID: "1",
1626-
ID: "1",
1627-
},
1628-
},
1629-
},
1630-
},
1631-
},
1632-
}
1633-
1634-
return payload
1635-
}
1636-
16371503
func samplePayloadWithoutIncluded() map[string]interface{} {
16381504
return map[string]interface{}{
16391505
"data": map[string]interface{}{

0 commit comments

Comments
 (0)