21.x backport 3525 max result nodes#3528
Conversation
| MergedField field = parameters.getField(); | ||
| GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); | ||
| GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field.getSingleField()); | ||
| GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry(); |
There was a problem hiding this comment.
This line is moved down in this PR
In latest master we added one extra method to break up the logic here. The code registry is used in the method being called here
| 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.
This has default visibility on current master
| 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.
This has default visibility on current master
dondonz
changed the title
| if ((maxNodes = executionContext.getGraphQLContext().get(MAX_RESULT_NODES)) != null) { | ||
| if (resultNodesCount > maxNodes) { | ||
| executionContext.getResultNodesInfo().maxResultNodesExceeded(); | ||
| return new FieldValueInfo(NULL, completedFuture(ExecutionResult.newExecutionResult().build()), fieldValueInfos); |
There was a problem hiding this comment.
In master this second argument is completedFuture(null). In this version it MUST be completedFuture(ExecutionResult.newExecutionResult().build())
This is because since 21.x was last released we have made a big change to remove unnecessary ExecutionResult objects. My test hung weirdly because in 21.x land, the engine never expects a null, it expects an ExecutionResult
| return CompletableFuture.completedFuture(new FetchedValue(null, null, ImmutableKit.emptyList(), null)); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The next two lines (variables field and parentType) are not part of max result backport. I have done this to make this method look as close as possible to master
| .queryDirectives(queryDirectives) | ||
| .build(); | ||
| }); | ||
| GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry(); |
There was a problem hiding this comment.
Here's where the codeRegistry line got moved to
| 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 has a slightly different constructor in v21 (compared to master), however this line still means effectively the same thing (return early with null)
2 participants
See the original PR #3525 for more details. This backports a max result nodes limit