diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index bf95d33..f9f3412 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2,8 +2,8 @@ # More on CODEOWNERS files: https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners # Default owners -* @hashicorp/tf-core-cloud * @hashicorp/team-ip-compliance +* @hashicorp/tf-core-cloud # Add override rules below. Each line is a file/folder pattern followed by one or more owners. # Being an owner means those groups or individuals will be added as reviewers to PRs affecting diff --git a/models_test.go b/models_test.go index 02e5d71..0961e1b 100644 --- a/models_test.go +++ b/models_test.go @@ -90,36 +90,15 @@ type GenericInterface struct { Data interface{} `jsonapi:"attr,interface"` } -type Organization struct { - ID int `jsonapi:"primary,organizations"` - ClientID string `jsonapi:"client-id"` - Name string `jsonapi:"attr,title"` - DefaultProject *Project `jsonapi:"relation,default_project"` - CreatedAt time.Time `jsonapi:"attr,created_at"` - - Links Links `jsonapi:"links,omitempty"` -} - -type Project struct { - ID int `jsonapi:"primary,projects"` - ClientID string `jsonapi:"client-id"` - Name string `jsonapi:"attr,name"` - Organization *Organization `jsonapi:"relation,organization"` - - Links Links `jsonapi:"links,omitempty"` -} - type Blog struct { - ID int `jsonapi:"primary,blogs"` - ClientID string `jsonapi:"client-id"` - Title string `jsonapi:"attr,title"` - CurrentPostID int `jsonapi:"attr,current_post_id"` - CreatedAt time.Time `jsonapi:"attr,created_at"` - ViewCount int `jsonapi:"attr,view_count"` - Posts []*Post `jsonapi:"relation,posts"` - CurrentPost *Post `jsonapi:"relation,current_post"` - Organization *Organization `jsonapi:"relation,organization"` - Project *Project `jsonapi:"relation,project"` + ID int `jsonapi:"primary,blogs"` + ClientID string `jsonapi:"client-id"` + Title string `jsonapi:"attr,title"` + Posts []*Post `jsonapi:"relation,posts"` + CurrentPost *Post `jsonapi:"relation,current_post"` + CurrentPostID int `jsonapi:"attr,current_post_id"` + CreatedAt time.Time `jsonapi:"attr,created_at"` + ViewCount int `jsonapi:"attr,view_count"` Links Links `jsonapi:"links,omitempty"` } diff --git a/request.go b/request.go index 48504dd..ea78c50 100644 --- a/request.go +++ b/request.go @@ -60,11 +60,6 @@ func newErrUnsupportedPtrType(rf reflect.Value, t reflect.Type, structField refl return ErrUnsupportedPtrType{rf, t, structField} } -type includedNode struct { - node *Node - model *reflect.Value -} - // UnmarshalPayload converts an io into a struct instance using jsonapi tags on // struct fields. This method supports single request payloads only, at the // moment. Bulk creates and updates are not supported yet. @@ -99,19 +94,19 @@ type includedNode struct { // model interface{} should be a pointer to a struct. func UnmarshalPayload(in io.Reader, model interface{}) error { payload := new(OnePayload) - included := make(map[string]*includedNode) if err := json.NewDecoder(in).Decode(payload); err != nil { return err } if payload.Included != nil { - for _, include := range payload.Included { - key := fmt.Sprintf("%s,%s", include.Type, include.ID) - included[key] = &includedNode{include, nil} + includedMap := make(map[string]*Node) + for _, included := range payload.Included { + key := fmt.Sprintf("%s,%s", included.Type, included.ID) + includedMap[key] = included } - return unmarshalNode(payload.Data, reflect.ValueOf(model), &included) + return unmarshalNode(payload.Data, reflect.ValueOf(model), &includedMap) } return unmarshalNode(payload.Data, reflect.ValueOf(model), nil) } @@ -125,19 +120,19 @@ func UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) { return nil, err } - models := []interface{}{} // will be populated from the "data" - included := map[string]*includedNode{} // will be populate from the "included" + models := []interface{}{} // will be populated from the "data" + includedMap := map[string]*Node{} // will be populate from the "included" if payload.Included != nil { - for _, include := range payload.Included { - key := fmt.Sprintf("%s,%s", include.Type, include.ID) - included[key] = &includedNode{include, nil} + for _, included := range payload.Included { + key := fmt.Sprintf("%s,%s", included.Type, included.ID) + includedMap[key] = included } } for _, data := range payload.Data { model := reflect.New(t.Elem()) - err := unmarshalNode(data, model, &included) + err := unmarshalNode(data, model, &includedMap) if err != nil { return nil, err } @@ -268,7 +263,7 @@ func getStructTags(field reflect.StructField) ([]string, error) { // unmarshalNodeMaybeChoice populates a model that may or may not be // a choice type struct that corresponds to a polyrelation or relation -func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, choiceTypeMapping map[string]structFieldIndex, included *map[string]*includedNode) error { +func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, choiceTypeMapping map[string]structFieldIndex, included *map[string]*Node) error { // This will hold either the value of the choice type model or the actual // model, depending on annotation var actualModel = *m @@ -305,7 +300,7 @@ func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, c return nil } -func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includedNode) (err error) { +func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) (err error) { defer func() { if r := recover(); r != nil { 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 continue } - // Check if the item in the relationship was already processed elsewhere. Avoids potential infinite recursive loops - // caused by circular references between included relationships (two included items include one another) - includedKey := fmt.Sprintf("%s,%s", relationship.Data.Type, relationship.Data.ID) - if included != nil && (*included)[includedKey] != nil { - if (*included)[includedKey].model != nil { - fieldValue.Set(*(*included)[includedKey].model) - } else { - (*included)[includedKey].model = &m - err := unmarshalNodeMaybeChoice(&m, (*included)[includedKey].node, annotation, choiceMapping, included) - if err != nil { - er = err - break - } - fieldValue.Set(m) - } - continue - } err = unmarshalNodeMaybeChoice(&m, relationship.Data, annotation, choiceMapping, included) if err != nil { er = err @@ -612,11 +590,11 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ return er } -func fullNode(n *Node, included *map[string]*includedNode) *Node { +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].node + return (*included)[includedKey] } return n diff --git a/request_test.go b/request_test.go index 56c407e..7fa0d3e 100644 --- a/request_test.go +++ b/request_test.go @@ -811,41 +811,6 @@ func TestUnmarshalRelationships(t *testing.T) { } } -func TestUnmarshalMany_relationships_with_circular_inclusion(t *testing.T) { - data := samplePayloadWithCircularInclusion() - payload, err := json.Marshal(data) - if err != nil { - t.Fatal(err) - } - in := bytes.NewReader(payload) - model := reflect.TypeOf(new(Blog)) - - out, err := UnmarshalManyPayload(in, model) - if err != nil { - t.Fatal(err) - } - - result_1 := out[0].(*Blog) - - if result_1.Project != result_1.Organization.DefaultProject { - t.Errorf("expected blog.project (%p) to hold the same pointer as blog.organization.default-project (%p) ", result_1.Project, result_1.Organization.DefaultProject) - } - - if result_1.Organization != result_1.Project.Organization { - t.Errorf("expected blog.organization (%p) to hold the same pointer as blog.project.organization (%p)", result_1.Organization, result_1.Project.Organization) - } - - result_2 := out[1].(*Blog) - - if result_2.Project != result_2.Organization.DefaultProject { - t.Errorf("expected blog.project (%p) to hold the same pointer as blog.organization.default-project (%p) ", result_2.Project, result_2.Organization.DefaultProject) - } - - if result_2.Organization != result_2.Project.Organization { - t.Errorf("expected blog.organization (%p) to hold the same pointer as blog.project.organization (%p)", result_2.Organization, result_2.Project.Organization) - } -} - func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { in := bytes.NewReader([]byte(`{ "data": { @@ -1535,105 +1500,6 @@ func TestUnmarshalCustomTypeAttributes_ErrInvalidType(t *testing.T) { } } -func samplePayloadWithCircularInclusion() *ManyPayload { - payload := &ManyPayload{ - Data: []*Node{ - { - Type: "blogs", - ClientID: "1", - ID: "1", - Attributes: map[string]interface{}{ - "title": "Foo", - "current_post_id": 1, - "created_at": 1436216820, - "view_count": 1000, - }, - Relationships: map[string]interface{}{ - "project": &RelationshipOneNode{ - Data: &Node{ - Type: "projects", - ClientID: "1", - ID: "1", - }, - }, - "organization": &RelationshipOneNode{ - Data: &Node{ - Type: "organizations", - ClientID: "1", - ID: "1", - }, - }, - }, - }, - { - Type: "blogs", - ClientID: "2", - ID: "2", - Attributes: map[string]interface{}{ - "title": "Foo2", - "current_post_id": 1, - "created_at": 1436216820, - "view_count": 1000, - }, - Relationships: map[string]interface{}{ - "project": &RelationshipOneNode{ - Data: &Node{ - Type: "projects", - ClientID: "1", - ID: "1", - }, - }, - "organization": &RelationshipOneNode{ - Data: &Node{ - Type: "organizations", - ClientID: "1", - ID: "1", - }, - }, - }, - }, - }, - Included: []*Node{ - { - Type: "projects", - ClientID: "1", - ID: "1", - Attributes: map[string]interface{}{ - "name": "Bar", - }, - Relationships: map[string]interface{}{ - "organization": &RelationshipOneNode{ - Data: &Node{ - Type: "organizations", - ClientID: "1", - ID: "1", - }, - }, - }, - }, - { - Type: "organizations", - ClientID: "1", - ID: "1", - Attributes: map[string]interface{}{ - "name": "Baz", - }, - Relationships: map[string]interface{}{ - "default_project": &RelationshipOneNode{ - Data: &Node{ - Type: "projects", - ClientID: "1", - ID: "1", - }, - }, - }, - }, - }, - } - - return payload -} - func samplePayloadWithoutIncluded() map[string]interface{} { return map[string]interface{}{ "data": map[string]interface{}{