Bug Fix: Avoid Stack Overflow when provided data model relationships are included and contain circular references

[Maed223]

Description of Bug

This fix relates to a bug seen downstream in go-tfe. In short, when the provided data model has circular references within it's relations AND those relations are specified in "included" we can run into stack overflows. Specifically if two included items have included relations to each other. Such a situation leads to the unmarshal process getting stuck in a endless recursive loop.

For a data model like so:

type Workspace struct {
	// Relations
	Organization                *Organization         `jsonapi:"relation,organization"`
	Project                     *Project              `jsonapi:"relation,project"`
}
type Organization struct {
	// Relations
	DefaultProject   *Project   `jsonapi:"relation,default-project"`
}
type Project struct {
	// Relations
	Organization *Organization `jsonapi:"relation,organization"`
}

When we specify the included as "project" and "organization", if within the included there is a project that is also the DefaultProject of an included Organization we see a call stack like so:

unmarshalNode(Workspace)
  → unmarshalNodeMaybeChoice(Project) 
    → unmarshalNode(Project)
      → unmarshalNodeMaybeChoice(Organization)
        → unmarshalNode(Organization)
          → unmarshalNodeMaybeChoice(DefaultProject/Project)
            → unmarshalNode(Project)  // And the cycle continues until we run out of memory...

As we forever try to render the included relationships.

The Fix

When unmarshalling a nodes relationships, if the relationship is given in the included, we store a pointer to the results of it's unmarshalling within the includedNode struct that is passed throughout the call stack. If this same item is in a relationship and included elsewhere, we opt to use the stored result to avoid the potential infinite loop we see above.

Testing

Added a test, with associated data model edits, that captures this behavior. If you wish to see the stack overflow that is caused without the fix, you can simply comment out this code section:

// This will hold either the value of the choice type model or the actual
				// model, depending on annotation
				m := reflect.New(fieldValue.Type().Elem())
                                 /**
				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
					break
				}

And run TestUnmarshalMany_relationships_with_circular_inclusion to see the stack overflow occur.

[brandonc]

Seems like a good idea to track the already-included models by id/type and avoid the recursion step if we already saw a particular record.

[mukeshjc]

LGTM