Skip to content

Coercing a null ExecutionInput query string to a persisted query marker #4008

@joshjcarrier

Description

@joshjcarrier

The problem

The current ExecutionInput object does not allow this representation:

private ExecutionInput(Builder builder) {
this.query = assertNotNull(builder.query, () -> "query can't be null");

and will throw an exception indicating "query" is required.

Support for Automatic Persisted Query

PersistedQuerySupport allows GraphQL clients that implement Apollo's Automatic Persisted Query to submit a GraphQL HTTP request body where the query body has been substituted with a smaller hash in extensions.persistedQuery.sha256Hash:

{
  "operationName": "...",
  "variables": {},
  "extensions": {
    "persistedQuery": {
      "version": 1,
      "sha256Hash": "...hash of query body"
   }
}

By setting a PreparsedDocumentProvider, the query can be retrieved by its sha256Hash:

GraphQL.newGraphQL(schema)
       .preparsedDocumentProvider(new ApolloPersistedQuerySupport(InMemoryPersistedQueryCache.newInMemoryPersistedQueryCache().build());

And if not found, the client can be challenged to send a non-APQ GraphQL request.

if (queryText == null || queryText.isBlank()) {
throw new PersistedQueryNotFound(persistedQueryId);
}

For this flow to be valid, graphql-java thus allows a null query as long as extensions.persistedQuery.sha256Hash is set.

The last recommendation

In 2021 it was the recommendation of #2636 that the caller provide "PersistedQueryMarker" via PersistedQuerySupport.PERSISTED_QUERY_MARKER instead of null.

/**
* In order for {@link graphql.ExecutionInput#getQuery()} to never be null, use this to mark
* them so that invariant can be satisfied while assuming that the persisted query id is elsewhere
*/
public static final String PERSISTED_QUERY_MARKER = "PersistedQueryMarker";

When to pass PERSISTED_QUERY_MARKER ?

Using graphql-java-servlet as an example, usage of ExecutionInput is pass-through of request parameters with minimal introspection:

https://github.com/graphql-java-kickstart/graphql-java-servlet/blob/1b69c873421b9aacd202f2c65c88a461f7250d9d/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/input/GraphQLSingleInvocationInput.java#L30-L42

    return ExecutionInput.newExecutionInput()
        .query(graphQLRequest.getQuery())
        .operationName(graphQLRequest.getOperationName())
        .context(context)
        .graphQLContext(context.getMapOfContext())
        .root(root)
        .variables(graphQLRequest.getVariables())
        .extensions(graphQLRequest.getExtensions())
        .dataLoaderRegistry(context.getDataLoaderRegistry())
        .executionId(ExecutionId.generate())
        .build();

To choose whether to substitute in the marker here (while still also preserving the true HTTP 400 Bad argument case) the caller has to know:

  • valid structures within extensions, as defined in graphql.execution.preparsed.persisted
  • which of those will be executed by graphql.GraphQL
  • ensure the extensions provided to the ExecutionInput.Builder matches the assumed values

Proposal: ExecutionInput's control

To match the stated use case

/**
* This represents the series of values that can be input on a graphql query execution
*/
@PublicApi
public class ExecutionInput {

perhaps the class could coerce query defaults, like it does for locale:

 /** 
  * This represents the series of values that can be input on a graphql query execution 
  */ 
public class ExecutionInput {
    private ExecutionInput(Builder builder) {
        this.query = assertQuery(builder);
        /// the rest
   }

   private static String assertQuery(Builder builder) {
     if ((builder.query == null || builder.query.isEmpty()) && builder.extensions.containsKey("persistedQuery")) {
       return PersistedQuerySupport.PERSISTED_QUERY_MARKER;
     }

     return assertNotNull(builder.query, () -> "query can't be null");
   }

   public static class Builder {
        public Builder query(String query) {
            this.query = query
            return this;
        }
  }

This can help hide the internal use of PERSISTED_QUERY_MARKER, and move towards a marker implementation configured by graphql.GraphQL.

Metadata

Metadata

Assignees

No one assigned

    Labels

    keep-openTells Stale Bot to keep PRs and issues open

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions