Skip to content

Allow recursive references in fragments selection subsets #929

Open
@rivantsov

Description

@rivantsov

Problem statement

GraphQL spec currently forbids recursive references in fragments - section 5.5.2.2:

5.5.2.2 Fragment spreads must not form cycles

The reasoning given is that reference cycles result in infinite loops - which is not always true. However, reference cycles, known as recursion in programming languages, can be useful in some situations, making GraphQL code shorter and cleaner while not causing any infinity problems.

Recursive references do not necessarily produce infinite loops. When a reference to a fragment (the owner fragment or other one forming a reference cycle) is inside a selection subset on one of the fields, the execution/unfolding the fragment(s) does not result in infinite loop (unless the data itself forms a loop at the same time, more on this later).

Example - type-unfolding fragment in GraphiQL introspection query

To unwrap complex type definitions for fields, the GraphiQL UI tool uses the following TypeRef fragment:

 fragment TypeRef on __Type {
      kind
      name
      ofType {
        kind
        name
        ofType {
          kind
          name
          ofType {
            kind
            name
            ofType {
              kind
              name
              ofType {
                kind
                name
                ofType {
                  kind
                  name
                  ofType {
                    kind
                    name
                  }
                }
              }
            }
          }
        }
      }
    }

Not pretty to put it softly; unfolds the wrapped types only to a fixed limited depth, thus making an assumption of max type complexity.

The same information could be retrieved with a simple recursive fragment, if recursion was allowed:

fragment TypeRefRec on __Type {
    kind
    name  
    ofType {
      ...TypeRefRec
    }
}

There are many common examples of data reference chains that can be unfolded using recursive fragments. For example the corp hierarchy, the sequence of 'reports-to' links for an employee in a company. Microsoft Outlook has a UI form that shows the employee's chain of managers up to the top CEO, as shown in this article (scroll down to the pic of Organization tab):

https://blog.hyperfish.com/get-the-most-out-of-your-organizational-charts-in-office-365

Retrieving the entire managers list can be easily done using a recursive fragment. Without recursion, we have to make an arbitrary assumption about number of subordination levels (which can be high in large corp like Microsoft), and then explicitly encoded it in a multi-level fragment.

As a general observation, most (all?) programming languages allow recursion, at least syntactically, and none to my knowledge forbid using it. GraphQL spec stands out as a strange outlier, especially considering obvious good use cases as shown above.

Suggested Change

Change the section in spec to allow servers to allow circular (recursive) references; leave it to server implementation, to ALLOW them in selection subsets (new behavior) or FORBID them outright (old behavior).

General note : In this and many other cases I feel like the spec tries to dictate too much and go into excessive details about implementation of some simple matters. Honestly, it feels like insult sometimes, as if I do not know the potential dangers of recursion and need these excessive explanations with examples. It's not just me, as a server developer; I feel we insult reader's intelligence in general, no matter his/her specific expertise. In the case of recursive fragments, instead of putting all this text and examples etc, we could just put a simple reminder - beware of circular refs, they can result in infinite resolution loops.

Potential Objections/Challenges

Recursive fragments can result in infinite loops if data references form a loop.
Fix: Can be easily detected by the server; the engine needs a counter of current fragment depth, and a check/circuit breaker terminating execution when this counter exceeds some reasonably high number. In general, it is much 'easier' to abuse a GraphQL server by a rogue client than RESTful API. So the GraphQL engine must necessarily have multiple circuit breakers to prevent abuse or excessive resource hogging by mistake. The extra counter for fragment depth is just one of multiple limiting counters inside the engine, and should not be a challenge.

Potential impact in breaking existing implementation

The impact should be very low or none at all. The suggested change is to ALLOW servers to ALLOW recursive fragments, but server implementers are free to forbid them as before. Thus old servers are automatically compatible with new spec.

Test implementation

The recursive fragments are implemented successfully in [NGraphQL] (https://github.com/rivantsov/ngraphql) framework. There is a unit test in the solution that uses a recursive TypeDetails fragment (slightly modified here, 'displayName' in actual test is another ext of standard irrelevant here):

fragment TypeDetails on __Type {
    name
    kind
    ofType {
      ...TypeDetails
    }
}

This fragment is used to unfold the type of the 'list' member:

input InputObjWithList {
  list: [[Int!]]!
}

The test printout (request and response) can be seen here:

https://github.com/rivantsov/ngraphql/blob/master/misc/UnitTestsLog.txt#L655

Note that the server uses static analysis of the query (pre-execution) to detect circular references at the top level (not in subsets) and rejects such queries outright. It uses run-time depth counter to detect infinity loops in data and returns runtime error.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions