Skip to content

Commit b424f98

Browse files
bbakermanbbakerman
authored
Max query depth called later in beginExecuteOperation (graphql-java#2773)
* Investigation of NonNullableValueCoercedAsNullException is being thrown in MaxQueryDepthInstrumentation * MaxQueryDepthInstrumentation and MaxQueryComplexityInstrumentation now are called at execution time not after validation * use atomic ref
1 parent 7086a3e commit b424f98

5 files changed

Lines changed: 193 additions & 220 deletions

File tree

‎src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java‎

Lines changed: 57 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
package graphql.analysis;
22

3+
import graphql.ExecutionResult;
34
import graphql.PublicApi;
45
import graphql.execution.AbortExecutionException;
6+
import graphql.execution.ExecutionContext;
57
import graphql.execution.instrumentation.InstrumentationContext;
8+
import graphql.execution.instrumentation.InstrumentationState;
69
import graphql.execution.instrumentation.SimpleInstrumentation;
10+
import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters;
11+
import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters;
712
import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters;
813
import graphql.validation.ValidationError;
914
import org.slf4j.Logger;
@@ -12,10 +17,11 @@
1217
import java.util.LinkedHashMap;
1318
import java.util.List;
1419
import java.util.Map;
20+
import java.util.concurrent.atomic.AtomicReference;
1521
import java.util.function.Function;
1622

1723
import static graphql.Assert.assertNotNull;
18-
import static graphql.execution.instrumentation.SimpleInstrumentationContext.whenCompleted;
24+
import static graphql.execution.instrumentation.SimpleInstrumentationContext.noOp;
1925
import static java.util.Optional.ofNullable;
2026

2127
/**
@@ -76,41 +82,52 @@ public MaxQueryComplexityInstrumentation(int maxComplexity, FieldComplexityCalcu
7682
this.maxQueryComplexityExceededFunction = maxQueryComplexityExceededFunction;
7783
}
7884

85+
@Override
86+
public InstrumentationState createState(InstrumentationCreateStateParameters parameters) {
87+
return new State();
88+
}
89+
7990
@Override
8091
public InstrumentationContext<List<ValidationError>> beginValidation(InstrumentationValidationParameters parameters) {
81-
return whenCompleted((errors, throwable) -> {
82-
if ((errors != null && errors.size() > 0) || throwable != null) {
83-
return;
84-
}
85-
QueryTraverser queryTraverser = newQueryTraverser(parameters);
86-
87-
Map<QueryVisitorFieldEnvironment, Integer> valuesByParent = new LinkedHashMap<>();
88-
queryTraverser.visitPostOrder(new QueryVisitorStub() {
89-
@Override
90-
public void visitField(QueryVisitorFieldEnvironment env) {
91-
int childsComplexity = valuesByParent.getOrDefault(env, 0);
92-
int value = calculateComplexity(env, childsComplexity);
93-
94-
valuesByParent.compute(env.getParentEnvironment(), (key, oldValue) ->
95-
ofNullable(oldValue).orElse(0) + value
96-
);
97-
}
98-
});
99-
int totalComplexity = valuesByParent.getOrDefault(null, 0);
100-
if (log.isDebugEnabled()) {
101-
log.debug("Query complexity: {}", totalComplexity);
102-
}
103-
if (totalComplexity > maxComplexity) {
104-
QueryComplexityInfo queryComplexityInfo = QueryComplexityInfo.newQueryComplexityInfo()
105-
.complexity(totalComplexity)
106-
.instrumentationValidationParameters(parameters)
107-
.build();
108-
boolean throwAbortException = maxQueryComplexityExceededFunction.apply(queryComplexityInfo);
109-
if (throwAbortException) {
110-
throw mkAbortException(totalComplexity, maxComplexity);
111-
}
92+
State state = parameters.getInstrumentationState();
93+
// for API backwards compatibility reasons we capture the validation parameters, so we can put them into QueryComplexityInfo
94+
state.instrumentationValidationParameters.set(parameters);
95+
return noOp();
96+
}
97+
98+
@Override
99+
public InstrumentationContext<ExecutionResult> beginExecuteOperation(InstrumentationExecuteOperationParameters instrumentationExecuteOperationParameters) {
100+
State state = instrumentationExecuteOperationParameters.getInstrumentationState();
101+
QueryTraverser queryTraverser = newQueryTraverser(instrumentationExecuteOperationParameters.getExecutionContext());
102+
103+
Map<QueryVisitorFieldEnvironment, Integer> valuesByParent = new LinkedHashMap<>();
104+
queryTraverser.visitPostOrder(new QueryVisitorStub() {
105+
@Override
106+
public void visitField(QueryVisitorFieldEnvironment env) {
107+
int childsComplexity = valuesByParent.getOrDefault(env, 0);
108+
int value = calculateComplexity(env, childsComplexity);
109+
110+
valuesByParent.compute(env.getParentEnvironment(), (key, oldValue) ->
111+
ofNullable(oldValue).orElse(0) + value
112+
);
112113
}
113114
});
115+
int totalComplexity = valuesByParent.getOrDefault(null, 0);
116+
if (log.isDebugEnabled()) {
117+
log.debug("Query complexity: {}", totalComplexity);
118+
}
119+
if (totalComplexity > maxComplexity) {
120+
QueryComplexityInfo queryComplexityInfo = QueryComplexityInfo.newQueryComplexityInfo()
121+
.complexity(totalComplexity)
122+
.instrumentationValidationParameters(state.instrumentationValidationParameters.get())
123+
.instrumentationExecuteOperationParameters(instrumentationExecuteOperationParameters)
124+
.build();
125+
boolean throwAbortException = maxQueryComplexityExceededFunction.apply(queryComplexityInfo);
126+
if (throwAbortException) {
127+
throw mkAbortException(totalComplexity, maxComplexity);
128+
}
129+
}
130+
return noOp();
114131
}
115132

116133
/**
@@ -125,12 +142,12 @@ protected AbortExecutionException mkAbortException(int totalComplexity, int maxC
125142
return new AbortExecutionException("maximum query complexity exceeded " + totalComplexity + " > " + maxComplexity);
126143
}
127144

128-
QueryTraverser newQueryTraverser(InstrumentationValidationParameters parameters) {
145+
QueryTraverser newQueryTraverser(ExecutionContext executionContext) {
129146
return QueryTraverser.newQueryTraverser()
130-
.schema(parameters.getSchema())
131-
.document(parameters.getDocument())
132-
.operationName(parameters.getOperation())
133-
.variables(parameters.getVariables())
147+
.schema(executionContext.getGraphQLSchema())
148+
.document(executionContext.getDocument())
149+
.operationName(executionContext.getExecutionInput().getOperationName())
150+
.variables(executionContext.getVariables())
134151
.build();
135152
}
136153

@@ -156,5 +173,8 @@ private FieldComplexityEnvironment convertEnv(QueryVisitorFieldEnvironment query
156173
);
157174
}
158175

176+
private static class State implements InstrumentationState {
177+
AtomicReference<InstrumentationValidationParameters> instrumentationValidationParameters = new AtomicReference<>();
178+
}
159179

160180
}

‎src/main/java/graphql/analysis/MaxQueryDepthInstrumentation.java‎

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
package graphql.analysis;
22

3+
import graphql.ExecutionResult;
34
import graphql.PublicApi;
45
import graphql.execution.AbortExecutionException;
6+
import graphql.execution.ExecutionContext;
57
import graphql.execution.instrumentation.InstrumentationContext;
68
import graphql.execution.instrumentation.SimpleInstrumentation;
7-
import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters;
8-
import graphql.validation.ValidationError;
9+
import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters;
910
import org.slf4j.Logger;
1011
import org.slf4j.LoggerFactory;
1112

12-
import java.util.List;
1313
import java.util.function.Function;
1414

15-
import static graphql.execution.instrumentation.SimpleInstrumentationContext.whenCompleted;
15+
import static graphql.execution.instrumentation.SimpleInstrumentationContext.noOp;
1616

1717
/**
1818
* Prevents execution if the query depth is greater than the specified maxDepth.
@@ -49,26 +49,22 @@ public MaxQueryDepthInstrumentation(int maxDepth, Function<QueryDepthInfo, Boole
4949
}
5050

5151
@Override
52-
public InstrumentationContext<List<ValidationError>> beginValidation(InstrumentationValidationParameters parameters) {
53-
return whenCompleted((errors, throwable) -> {
54-
if ((errors != null && errors.size() > 0) || throwable != null) {
55-
return;
56-
}
57-
QueryTraverser queryTraverser = newQueryTraverser(parameters);
58-
int depth = queryTraverser.reducePreOrder((env, acc) -> Math.max(getPathLength(env.getParentEnvironment()), acc), 0);
59-
if (log.isDebugEnabled()) {
60-
log.debug("Query depth info: {}", depth);
61-
}
62-
if (depth > maxDepth) {
63-
QueryDepthInfo queryDepthInfo = QueryDepthInfo.newQueryDepthInfo()
64-
.depth(depth)
65-
.build();
66-
boolean throwAbortException = maxQueryDepthExceededFunction.apply(queryDepthInfo);
67-
if (throwAbortException) {
68-
throw mkAbortException(depth, maxDepth);
69-
}
52+
public InstrumentationContext<ExecutionResult> beginExecuteOperation(InstrumentationExecuteOperationParameters parameters) {
53+
QueryTraverser queryTraverser = newQueryTraverser(parameters.getExecutionContext());
54+
int depth = queryTraverser.reducePreOrder((env, acc) -> Math.max(getPathLength(env.getParentEnvironment()), acc), 0);
55+
if (log.isDebugEnabled()) {
56+
log.debug("Query depth info: {}", depth);
57+
}
58+
if (depth > maxDepth) {
59+
QueryDepthInfo queryDepthInfo = QueryDepthInfo.newQueryDepthInfo()
60+
.depth(depth)
61+
.build();
62+
boolean throwAbortException = maxQueryDepthExceededFunction.apply(queryDepthInfo);
63+
if (throwAbortException) {
64+
throw mkAbortException(depth, maxDepth);
7065
}
71-
});
66+
}
67+
return noOp();
7268
}
7369

7470
/**
@@ -83,12 +79,12 @@ protected AbortExecutionException mkAbortException(int depth, int maxDepth) {
8379
return new AbortExecutionException("maximum query depth exceeded " + depth + " > " + maxDepth);
8480
}
8581

86-
QueryTraverser newQueryTraverser(InstrumentationValidationParameters parameters) {
82+
QueryTraverser newQueryTraverser(ExecutionContext executionContext) {
8783
return QueryTraverser.newQueryTraverser()
88-
.schema(parameters.getSchema())
89-
.document(parameters.getDocument())
90-
.operationName(parameters.getOperation())
91-
.variables(parameters.getVariables())
84+
.schema(executionContext.getGraphQLSchema())
85+
.document(executionContext.getDocument())
86+
.operationName(executionContext.getExecutionInput().getOperationName())
87+
.variables(executionContext.getVariables())
9288
.build();
9389
}
9490

‎src/main/java/graphql/analysis/QueryComplexityInfo.java‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.analysis;
22

33
import graphql.PublicApi;
4+
import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters;
45
import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters;
56

67
/**
@@ -10,11 +11,13 @@
1011
public class QueryComplexityInfo {
1112

1213
private final int complexity;
13-
private InstrumentationValidationParameters instrumentationValidationParameters;
14+
private final InstrumentationValidationParameters instrumentationValidationParameters;
15+
private final InstrumentationExecuteOperationParameters instrumentationExecuteOperationParameters;
1416

15-
private QueryComplexityInfo(int complexity, InstrumentationValidationParameters parameters) {
16-
this.complexity = complexity;
17-
this.instrumentationValidationParameters = parameters;
17+
private QueryComplexityInfo(Builder builder) {
18+
this.complexity = builder.complexity;
19+
this.instrumentationValidationParameters = builder.instrumentationValidationParameters;
20+
this.instrumentationExecuteOperationParameters = builder.instrumentationExecuteOperationParameters;
1821
}
1922

2023
/**
@@ -35,6 +38,15 @@ public InstrumentationValidationParameters getInstrumentationValidationParameter
3538
return instrumentationValidationParameters;
3639
}
3740

41+
/**
42+
* This returns the instrumentation execute operation parameters.
43+
*
44+
* @return the instrumentation execute operation parameters.
45+
*/
46+
public InstrumentationExecuteOperationParameters getInstrumentationExecuteOperationParameters() {
47+
return instrumentationExecuteOperationParameters;
48+
}
49+
3850
@Override
3951
public String toString() {
4052
return "QueryComplexityInfo{" +
@@ -54,6 +66,7 @@ public static class Builder {
5466

5567
private int complexity;
5668
private InstrumentationValidationParameters instrumentationValidationParameters;
69+
private InstrumentationExecuteOperationParameters instrumentationExecuteOperationParameters;
5770

5871
private Builder() {
5972
}
@@ -62,6 +75,7 @@ private Builder() {
6275
* The query complexity.
6376
*
6477
* @param complexity the query complexity
78+
*
6579
* @return this builder
6680
*/
6781
public Builder complexity(int complexity) {
@@ -73,18 +87,24 @@ public Builder complexity(int complexity) {
7387
* The instrumentation validation parameters.
7488
*
7589
* @param parameters the instrumentation validation parameters.
90+
*
7691
* @return this builder
7792
*/
7893
public Builder instrumentationValidationParameters(InstrumentationValidationParameters parameters) {
7994
this.instrumentationValidationParameters = parameters;
8095
return this;
8196
}
8297

98+
public Builder instrumentationExecuteOperationParameters(InstrumentationExecuteOperationParameters instrumentationExecuteOperationParameters) {
99+
this.instrumentationExecuteOperationParameters = instrumentationExecuteOperationParameters;
100+
return this;
101+
}
102+
83103
/**
84104
* @return a built {@link QueryComplexityInfo} object
85105
*/
86106
public QueryComplexityInfo build() {
87-
return new QueryComplexityInfo(complexity, instrumentationValidationParameters);
107+
return new QueryComplexityInfo(this);
88108
}
89109
}
90110
}

0 commit comments

Comments
 (0)