Coercing a null ExecutionInput query string to a persisted query marker
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:
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
extensionsprovided to theExecutionInput.Buildermatches 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.