Skip to content

Commit abb2cdd

Browse files
dondonzdondonz
authored
Merge pull request #3929 from graphql-java/dont-alllocate-NonNullableFieldValidator-all-the-time
Stop creating NonNullableFieldValidator every for every object or list field
2 parents 0ad655c + 80f4a3b commit abb2cdd

10 files changed

Lines changed: 66 additions & 50 deletions

‎src/main/java/graphql/execution/Execution.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ private CompletableFuture<ExecutionResult> executeOperation(ExecutionContext exe
189189

190190
ResultPath path = ResultPath.rootPath();
191191
ExecutionStepInfo executionStepInfo = newExecutionStepInfo().type(operationRootType).path(path).build();
192-
NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo);
192+
NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext);
193193

194194
ExecutionStrategyParameters parameters = newParameters()
195195
.executionStepInfo(executionStepInfo)

‎src/main/java/graphql/execution/ExecutionStrategy.java‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -605,10 +605,7 @@ private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionC
605605
instrumentationParams, executionContext.getInstrumentationState()
606606
));
607607

608-
NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo);
609-
610608
ExecutionStrategyParameters newParameters = parameters.transform(executionStepInfo,
611-
nonNullableFieldValidator,
612609
fetchedValue.getLocalContext(),
613610
fetchedValue.getFetchedValue());
614611

@@ -767,13 +764,12 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext,
767764

768765
ExecutionStepInfo stepInfoForListElement = executionStepInfoFactory.newExecutionStepInfoForListElement(executionStepInfo, indexedPath);
769766

770-
NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, stepInfoForListElement);
771-
772767
FetchedValue value = unboxPossibleDataFetcherResult(executionContext, parameters, item);
773768

774769
ExecutionStrategyParameters newParameters = parameters.transform(stepInfoForListElement,
775-
nonNullableFieldValidator, indexedPath,
776-
value.getLocalContext(), value.getFetchedValue());
770+
indexedPath,
771+
value.getLocalContext(),
772+
value.getFetchedValue());
777773

778774
fieldValueInfos.add(completeValue(executionContext, newParameters));
779775
index++;
@@ -910,10 +906,8 @@ protected Object completeValueForObject(ExecutionContext executionContext, Execu
910906
);
911907

912908
ExecutionStepInfo newExecutionStepInfo = executionStepInfo.changeTypeWithPreservedNonNull(resolvedObjectType);
913-
NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, newExecutionStepInfo);
914909

915910
ExecutionStrategyParameters newParameters = parameters.transform(newExecutionStepInfo,
916-
nonNullableFieldValidator,
917911
subFields,
918912
result);
919913

‎src/main/java/graphql/execution/ExecutionStrategyParameters.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ private ExecutionStrategyParameters(ExecutionStepInfo executionStepInfo,
3838
this.localContext = localContext;
3939
this.fields = assertNotNull(fields, () -> "fields is null");
4040
this.source = source;
41-
this.nonNullableFieldValidator = nonNullableFieldValidator;
41+
this.nonNullableFieldValidator = assertNotNull(nonNullableFieldValidator, () -> "requires a NonNullValidator");;
4242
this.path = path;
4343
this.currentField = currentField;
4444
this.parent = parent;
@@ -132,7 +132,6 @@ ExecutionStrategyParameters transform(MergedField currentField,
132132

133133
@Internal
134134
ExecutionStrategyParameters transform(ExecutionStepInfo executionStepInfo,
135-
NonNullableFieldValidator nonNullableFieldValidator,
136135
MergedSelectionSet fields,
137136
Object source) {
138137
return new ExecutionStrategyParameters(executionStepInfo,
@@ -148,7 +147,6 @@ ExecutionStrategyParameters transform(ExecutionStepInfo executionStepInfo,
148147

149148
@Internal
150149
ExecutionStrategyParameters transform(ExecutionStepInfo executionStepInfo,
151-
NonNullableFieldValidator nonNullableFieldValidator,
152150
ResultPath path,
153151
Object localContext,
154152
Object source) {
@@ -165,7 +163,6 @@ ExecutionStrategyParameters transform(ExecutionStepInfo executionStepInfo,
165163

166164
@Internal
167165
ExecutionStrategyParameters transform(ExecutionStepInfo executionStepInfo,
168-
NonNullableFieldValidator nonNullableFieldValidator,
169166
Object localContext,
170167
Object source) {
171168
return new ExecutionStrategyParameters(executionStepInfo,

‎src/main/java/graphql/execution/NonNullableFieldValidator.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,13 @@
1414
public class NonNullableFieldValidator {
1515

1616
private final ExecutionContext executionContext;
17-
private final ExecutionStepInfo executionStepInfo;
1817

19-
public NonNullableFieldValidator(ExecutionContext executionContext, ExecutionStepInfo executionStepInfo) {
18+
public NonNullableFieldValidator(ExecutionContext executionContext) {
2019
this.executionContext = executionContext;
21-
this.executionStepInfo = executionStepInfo;
2220
}
2321

2422
/**
25-
* Called to check that a value is non null if the type requires it to be non null
23+
* Called to check that a value is non-null if the type requires it to be non null
2624
*
2725
* @param parameters the execution strategy parameters
2826
* @param result the result to check
@@ -34,6 +32,7 @@ public NonNullableFieldValidator(ExecutionContext executionContext, ExecutionSte
3432
*/
3533
public <T> T validate(ExecutionStrategyParameters parameters, T result) throws NonNullableFieldWasNullException {
3634
if (result == null) {
35+
ExecutionStepInfo executionStepInfo = parameters.getExecutionStepInfo();
3736
if (executionStepInfo.isNonNullType()) {
3837
// see https://spec.graphql.org/October2021/#sec-Errors-and-Non-Nullability
3938
//

‎src/main/java/graphql/execution/SubscriptionExecutionStrategy.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ private boolean keepOrdered(GraphQLContext graphQLContext) {
106106
*/
107107

108108
private CompletableFuture<Publisher<Object>> createSourceEventStream(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
109-
ExecutionStrategyParameters newParameters = firstFieldOfSubscriptionSelection(parameters);
109+
ExecutionStrategyParameters newParameters = firstFieldOfSubscriptionSelection(executionContext,parameters);
110110

111111
CompletableFuture<FetchedValue> fieldFetched = Async.toCompletableFuture(fetchField(executionContext, newParameters));
112112
return fieldFetched.thenApply(fetchedValue -> {
@@ -139,7 +139,7 @@ private CompletableFuture<ExecutionResult> executeSubscriptionEvent(ExecutionCon
139139
.root(eventPayload)
140140
.resetErrors()
141141
);
142-
ExecutionStrategyParameters newParameters = firstFieldOfSubscriptionSelection(parameters);
142+
ExecutionStrategyParameters newParameters = firstFieldOfSubscriptionSelection(newExecutionContext, parameters);
143143
ExecutionStepInfo subscribedFieldStepInfo = createSubscribedFieldStepInfo(executionContext, newParameters);
144144

145145
InstrumentationFieldParameters i13nFieldParameters = new InstrumentationFieldParameters(executionContext, () -> subscribedFieldStepInfo);
@@ -179,12 +179,14 @@ private String getRootFieldName(ExecutionStrategyParameters parameters) {
179179
return rootField.getResultKey();
180180
}
181181

182-
private ExecutionStrategyParameters firstFieldOfSubscriptionSelection(ExecutionStrategyParameters parameters) {
182+
private ExecutionStrategyParameters firstFieldOfSubscriptionSelection(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
183183
MergedSelectionSet fields = parameters.getFields();
184184
MergedField firstField = fields.getSubField(fields.getKeys().get(0));
185185

186186
ResultPath fieldPath = parameters.getPath().segment(mkNameForPath(firstField.getSingleField()));
187-
return parameters.transform(firstField,fieldPath);
187+
NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext);
188+
return parameters.transform(builder -> builder
189+
.field(firstField).path(fieldPath).nonNullFieldValidator(nonNullableFieldValidator));
188190
}
189191

190192
private ExecutionStepInfo createSubscribedFieldStepInfo(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {

‎src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ abstract class AsyncExecutionStrategyTest extends Specification {
117117
.newParameters()
118118
.executionStepInfo(typeInfo)
119119
.fields(mergedSelectionSet(['hello': mergedField([Field.newField('hello').build()]), 'hello2': mergedField([Field.newField('hello2').build()])]))
120+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
120121
.build()
121122

122123
AsyncExecutionStrategy asyncExecutionStrategy = new AsyncExecutionStrategy()
@@ -160,6 +161,7 @@ abstract class AsyncExecutionStrategyTest extends Specification {
160161
.newParameters()
161162
.executionStepInfo(typeInfo)
162163
.fields(mergedSelectionSet(['hello': mergedField([Field.newField('hello').build()]), 'hello2': mergedField([Field.newField('hello2').build()])]))
164+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
163165
.build()
164166

165167
AsyncExecutionStrategy asyncExecutionStrategy = new AsyncExecutionStrategy()
@@ -205,6 +207,7 @@ abstract class AsyncExecutionStrategyTest extends Specification {
205207
.newParameters()
206208
.executionStepInfo(typeInfo)
207209
.fields(mergedSelectionSet(['hello': mergedField([Field.newField('hello').build()]), 'hello2': mergedField([Field.newField('hello2').build()])]))
210+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
208211
.build()
209212

210213
AsyncExecutionStrategy asyncExecutionStrategy = new AsyncExecutionStrategy()
@@ -249,6 +252,7 @@ abstract class AsyncExecutionStrategyTest extends Specification {
249252
.newParameters()
250253
.executionStepInfo(typeInfo)
251254
.fields(mergedSelectionSet(['hello': mergedField([Field.newField('hello').build()]), 'hello2': mergedField([Field.newField('hello2').build()])]))
255+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
252256
.build()
253257

254258
AsyncExecutionStrategy asyncExecutionStrategy = new AsyncExecutionStrategy()
@@ -312,6 +316,7 @@ abstract class AsyncExecutionStrategyTest extends Specification {
312316
.newParameters()
313317
.executionStepInfo(typeInfo)
314318
.fields(mergedSelectionSet(['hello': mergedField([new Field('hello')]), 'hello2': mergedField([new Field('hello2')])]))
319+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
315320
.build()
316321

317322
AsyncExecutionStrategy asyncExecutionStrategy = new AsyncExecutionStrategy()

‎src/test/groovy/graphql/execution/AsyncSerialExecutionStrategyTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ class AsyncSerialExecutionStrategyTest extends Specification {
115115
.newParameters()
116116
.executionStepInfo(typeInfo)
117117
.fields(mergedSelectionSet(['hello': mergedField(new Field('hello')), 'hello2': mergedField(new Field('hello2')), 'hello3': mergedField(new Field('hello3'))]))
118+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
118119
.build()
119120

120121
AsyncSerialExecutionStrategy strategy = new AsyncSerialExecutionStrategy()
@@ -163,6 +164,7 @@ class AsyncSerialExecutionStrategyTest extends Specification {
163164
.newParameters()
164165
.executionStepInfo(typeInfo)
165166
.fields(mergedSelectionSet(['hello': mergedField(new Field('hello')), 'hello2': mergedField(new Field('hello2')), 'hello3': mergedField(new Field('hello3'))]))
167+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
166168
.build()
167169

168170
AsyncSerialExecutionStrategy strategy = new AsyncSerialExecutionStrategy()

‎src/test/groovy/graphql/execution/ExecutionStrategyParametersTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ class ExecutionStrategyParametersTest extends Specification {
1212

1313
def "ExecutionParameters can be transformed"() {
1414
given:
15+
def executionContext = Mock(ExecutionContext)
1516
def parameters = newParameters()
1617
.executionStepInfo(newExecutionStepInfo().type(GraphQLString))
1718
.source(new Object())
1819
.localContext("localContext")
20+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
1921
.fields(mergedSelectionSet("a": []))
2022
.build()
2123

0 commit comments

Comments
 (0)