19.x Backport PR 3525 max result nodes #3537
Conversation
| package graphql.execution | ||
|
|
||
|
|
||
| import graphql.GraphQLContext |
There was a problem hiding this comment.
Test changes: all tests instantiating a ExecutionContext ought to have had an empty or mocked GraphQLContext object added
In practice, during execution there is always an GraphQLContext inside the ExecutionContext, which is why we don't have a billion null checks for this in the engine. These tests have been brought up to date
|
|
||
| import graphql.ErrorType | ||
| import graphql.ExecutionResult | ||
| import graphql.GraphQLContext |
There was a problem hiding this comment.
Test changes: all tests instantiating a ExecutionContext ought to have had an empty or mocked GraphQLContext object added
In practice, during execution there is always an GraphQLContext inside the ExecutionContext, which is why we don't have a billion null checks for this in the engine. These tests have been brought up to date
In this file I use a Mock and in the next I use the default GraphQLContext. They mean the same, but I wanted to keep these files as close to master as possible. A Mock is used in this file in latest master to mock a field for the new defer feature.
| */ | ||
| public static GraphQLContext getDefault() { | ||
| return GraphQLContext.newContext().build(); | ||
| } |
There was a problem hiding this comment.
This convenience method was added in a later version - backported to help with this PR
| private final Object localContext; | ||
| private final ImmutableList<GraphQLError> errors; | ||
|
|
||
| private FetchedValue(Object fetchedValue, Object rawFetchedValue, ImmutableList<GraphQLError> errors, Object localContext) { |
There was a problem hiding this comment.
Changed visibility to be same as master
| private final CompletableFuture<ExecutionResult> fieldValue; | ||
| private final List<FieldValueInfo> fieldValueInfos; | ||
|
|
||
| private FieldValueInfo(CompleteValueType completeValueType, CompletableFuture<ExecutionResult> fieldValue, List<FieldValueInfo> fieldValueInfos) { |
There was a problem hiding this comment.
Changed visibility to be same as master
| if ((maxNodes = executionContext.getGraphQLContext().get(MAX_RESULT_NODES)) != null) { | ||
| if (resultNodesCount > maxNodes) { | ||
| executionContext.getResultNodesInfo().maxResultNodesExceeded(); | ||
| return CompletableFuture.completedFuture(new FetchedValue(null, null, ImmutableKit.emptyList(), null)); |
There was a problem hiding this comment.
FetchedValue constructor (and object) slightly different here vs master - there is one more field. But the meaning is the same, return early with a null
| .queryDirectives(queryDirectives) | ||
| .build(); | ||
|
|
||
| GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry(); |
| if ((maxNodes = executionContext.getGraphQLContext().get(MAX_RESULT_NODES)) != null) { | ||
| if (resultNodesCount > maxNodes) { | ||
| executionContext.getResultNodesInfo().maxResultNodesExceeded(); | ||
| return new FieldValueInfo(NULL, completedFuture(ExecutionResultImpl.newExecutionResult().build()), fieldValueInfos); |
There was a problem hiding this comment.
Difference to master - must have a completed value of type ExecutionResult. Can't return a plain null here or the engine is going to hang because prior to forthcoming v22 changes, the engine previously always expected an ExecutionResult
v19 is different to the other backports. This builder moved files after v19
Backports PR #3525, which enables setting a maximum number of result nodes