Skip to content

Commit a71d072

Browse files
tlovett-rmnbbakerman
tlovett-rmn
authored and
bbakerman
committed
Refactor GraphQL#execute for better encapsulation and clearer instrumentation lifecycles
1 parent 9f0876f commit a71d072

6 files changed

Lines changed: 112 additions & 36 deletions

File tree

‎src/main/java/graphql/ExecutionResultImpl.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33

44
import java.util.ArrayList;
5+
import java.util.Collections;
56
import java.util.List;
67

78
@Internal
@@ -10,6 +11,10 @@ public class ExecutionResultImpl implements ExecutionResult {
1011
private final List<GraphQLError> errors = new ArrayList<>();
1112
private Object data;
1213

14+
public ExecutionResultImpl(GraphQLError error) {
15+
this(Collections.singletonList(error));
16+
}
17+
1318
public ExecutionResultImpl(List<? extends GraphQLError> errors) {
1419
this(null, errors);
1520
}
@@ -23,8 +28,6 @@ public ExecutionResultImpl(Object data, List<? extends GraphQLError> errors) {
2328

2429
}
2530

26-
27-
2831
@Override
2932
public <T> T getData() {
3033
//noinspection unchecked

‎src/main/java/graphql/GraphQL.java‎

Lines changed: 81 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.slf4j.Logger;
2222
import org.slf4j.LoggerFactory;
2323

24-
import java.util.Collections;
2524
import java.util.List;
2625
import java.util.Map;
2726

@@ -280,50 +279,106 @@ public ExecutionResult execute(String query, String operationName, Object contex
280279
* @return result including errors
281280
*/
282281
public ExecutionResult execute(ExecutionInput executionInput) {
283-
String query = executionInput.getQuery();
284-
String operationName = executionInput.getOperationName();
285-
Object context = executionInput.getContext();
286-
Object root = executionInput.getRoot();
287-
Map<String, Object> variables = executionInput.getVariables() != null ? executionInput.getVariables() : Collections.emptyMap();
282+
log.debug("Executing request. operation name: {}. query: {} ", executionInput.getOperationName(), executionInput.getQuery());
288283

289-
log.debug("Executing request. operation name: {}. query: {} ", operationName, query);
284+
InstrumentationContext<ExecutionResult> executionInstrumentation = instrumentation.beginExecution(new InstrumentationExecutionParameters(executionInput));
285+
final ExecutionResult executionResult = parseValidateAndExecute(executionInput);
286+
executionInstrumentation.onEnd(executionResult);
287+
288+
return executionResult;
289+
}
290+
291+
private ExecutionResult parseValidateAndExecute(ExecutionInput executionInput) {
292+
ParseResult parseResult = parse(executionInput);
293+
if (parseResult.isFailure()) {
294+
return toParseFailureExecutionResult(parseResult.getException());
295+
}
296+
final Document document = parseResult.getDocument();
297+
298+
final List<ValidationError> errors = validate(executionInput, document);
299+
if (!errors.isEmpty()) {
300+
return new ExecutionResultImpl(errors);
301+
}
290302

291-
InstrumentationContext<ExecutionResult> executionCtx = instrumentation.beginExecution(new InstrumentationExecutionParameters(query, operationName, context, variables));
303+
return execute(executionInput, document);
304+
}
292305

306+
private ParseResult parse(ExecutionInput executionInput) {
307+
InstrumentationContext<Document> parseInstrumentation = instrumentation.beginParse(new InstrumentationExecutionParameters(executionInput));
293308

294-
InstrumentationContext<Document> parseCtx = instrumentation.beginParse(new InstrumentationExecutionParameters(query, operationName, context, variables));
295309
Parser parser = new Parser();
296310
Document document;
297311
try {
298-
document = parser.parseDocument(query);
299-
parseCtx.onEnd(document);
312+
document = parser.parseDocument(executionInput.getQuery());
300313
} catch (ParseCancellationException e) {
301-
RecognitionException recognitionException = (RecognitionException) e.getCause();
302-
SourceLocation sourceLocation = null;
303-
if (recognitionException != null) {
304-
sourceLocation = new SourceLocation(recognitionException.getOffendingToken().getLine(), recognitionException.getOffendingToken().getCharPositionInLine());
305-
}
306-
InvalidSyntaxError invalidSyntaxError = new InvalidSyntaxError(sourceLocation);
307-
return new ExecutionResultImpl(Collections.singletonList(invalidSyntaxError));
314+
parseInstrumentation.onEnd(e);
315+
return ParseResult.ofError((RecognitionException) e.getCause());
308316
}
309317

310-
InstrumentationContext<List<ValidationError>> validationCtx = instrumentation.beginValidation(new InstrumentationValidationParameters(query, operationName, context, variables, document));
318+
parseInstrumentation.onEnd(document);
319+
return ParseResult.of(document);
320+
}
321+
322+
private List<ValidationError> validate(ExecutionInput executionInput, Document document) {
323+
InstrumentationContext<List<ValidationError>> validationCtx = instrumentation.beginValidation(new InstrumentationValidationParameters(executionInput, document));
311324

312325
Validator validator = new Validator();
313326
List<ValidationError> validationErrors = validator.validateDocument(graphQLSchema, document);
314327

315328
validationCtx.onEnd(validationErrors);
329+
return validationErrors;
330+
}
316331

317-
if (validationErrors.size() > 0) {
318-
return new ExecutionResultImpl(validationErrors);
319-
}
320-
ExecutionId executionId = idProvider.provide(query, operationName, context);
332+
private ExecutionResult execute(ExecutionInput executionInput, Document document) {
333+
String query = executionInput.getQuery();
334+
String operationName = executionInput.getOperationName();
335+
Object context = executionInput.getContext();
321336

322337
Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation);
323-
ExecutionResult result = execution.execute(executionId, graphQLSchema, context, root, document, operationName, variables);
338+
ExecutionId executionId = idProvider.provide(query, operationName, context);
339+
return execution.execute(document, graphQLSchema, executionId, executionInput);
340+
}
341+
342+
private ExecutionResult toParseFailureExecutionResult(RecognitionException exception) {
343+
InvalidSyntaxError invalidSyntaxError = toInvalidSyntaxError(exception);
344+
return new ExecutionResultImpl(invalidSyntaxError);
345+
}
346+
347+
private InvalidSyntaxError toInvalidSyntaxError(RecognitionException recognitionException) {
348+
SourceLocation sourceLocation = null;
349+
if (recognitionException != null) {
350+
sourceLocation = new SourceLocation(recognitionException.getOffendingToken().getLine(), recognitionException.getOffendingToken().getCharPositionInLine());
351+
}
352+
return new InvalidSyntaxError(sourceLocation);
353+
}
354+
355+
private static class ParseResult {
356+
private final Document document;
357+
private final RecognitionException exception;
358+
359+
private ParseResult(Document document, RecognitionException exception) {
360+
this.document = document;
361+
this.exception = exception;
362+
}
324363

325-
executionCtx.onEnd(result);
364+
private boolean isFailure() {
365+
return document == null;
366+
}
326367

327-
return result;
368+
private Document getDocument() {
369+
return document;
370+
}
371+
372+
private RecognitionException getException() {
373+
return exception;
374+
}
375+
376+
private static ParseResult of(Document document) {
377+
return new ParseResult(document, null);
378+
}
379+
380+
private static ParseResult ofError(RecognitionException e) {
381+
return new ParseResult(null, e);
382+
}
328383
}
329384
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.execution;
22

33

4+
import graphql.ExecutionInput;
45
import graphql.ExecutionResult;
56
import graphql.ExecutionResultImpl;
67
import graphql.GraphQLException;
@@ -41,7 +42,12 @@ public Execution(ExecutionStrategy queryStrategy, ExecutionStrategy mutationStra
4142
this.instrumentation = instrumentation;
4243
}
4344

44-
public ExecutionResult execute(ExecutionId executionId, GraphQLSchema graphQLSchema, Object context, Object root, Document document, String operationName, Map<String, Object> variables) {
45+
public ExecutionResult execute(Document document, GraphQLSchema graphQLSchema, ExecutionId executionId, ExecutionInput executionInput) {
46+
final Object context = executionInput.getContext();
47+
final Object root = executionInput.getRoot();
48+
final String operationName = executionInput.getOperationName();
49+
final Map<String, Object> variables = executionInput.getVariables();
50+
4551
ExecutionContextBuilder executionContextBuilder = new ExecutionContextBuilder(new ValuesResolver(), instrumentation);
4652
ExecutionContext executionContext = executionContextBuilder
4753
.executionId(executionId)

‎src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionParameters.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package graphql.execution.instrumentation.parameters;
22

3+
import graphql.ExecutionInput;
34
import graphql.PublicApi;
45
import graphql.execution.instrumentation.Instrumentation;
56

7+
import java.util.Collections;
68
import java.util.Map;
79

810
/**
@@ -15,6 +17,15 @@ public class InstrumentationExecutionParameters {
1517
private final Object context;
1618
private final Map<String, Object> variables;
1719

20+
public InstrumentationExecutionParameters(ExecutionInput executionInput) {
21+
this(
22+
executionInput.getQuery(),
23+
executionInput.getOperationName(),
24+
executionInput.getContext(),
25+
executionInput.getVariables() != null ? executionInput.getVariables() : Collections.emptyMap()
26+
);
27+
}
28+
1829
public InstrumentationExecutionParameters(String query, String operation, Object context, Map<String, Object> variables) {
1930
this.query = query;
2031
this.operation = operation;

‎src/main/java/graphql/execution/instrumentation/parameters/InstrumentationValidationParameters.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
package graphql.execution.instrumentation.parameters;
22

3+
import graphql.ExecutionInput;
34
import graphql.execution.instrumentation.Instrumentation;
45
import graphql.language.Document;
56

6-
import java.util.Map;
7-
87
/**
98
* Parameters sent to {@link Instrumentation} methods
109
*/
1110
public class InstrumentationValidationParameters extends InstrumentationExecutionParameters {
1211
private final Document document;
1312

14-
public InstrumentationValidationParameters(String query, String operation, Object context, Map<String, Object> arguments, Document document) {
15-
super(query, operation, context, arguments);
13+
public InstrumentationValidationParameters(ExecutionInput executionInput, Document document) {
14+
super(executionInput);
1615
this.document = document;
1716
}
1817

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package graphql.execution
22

3+
import graphql.ExecutionInput
34
import graphql.MutationSchema
45
import graphql.execution.instrumentation.NoOpInstrumentation
56
import graphql.parser.Parser
@@ -12,6 +13,7 @@ class ExecutionTest extends Specification {
1213
def mutationStrategy = Mock(ExecutionStrategy)
1314
def queryStrategy = Mock(ExecutionStrategy)
1415
def execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, NoOpInstrumentation.INSTANCE)
16+
def emptyExecutionInput = ExecutionInput.newExecutionInput().build()
1517

1618
def "query strategy is used for query requests"() {
1719
given:
@@ -30,7 +32,7 @@ class ExecutionTest extends Specification {
3032
def document = parser.parseDocument(query)
3133

3234
when:
33-
execution.execute(ExecutionId.generate(), MutationSchema.schema, null, null, document, null, null)
35+
execution.execute(document, MutationSchema.schema, ExecutionId.generate(), emptyExecutionInput)
3436

3537
then:
3638
1 * queryStrategy.execute(*_)
@@ -50,7 +52,7 @@ class ExecutionTest extends Specification {
5052
def document = parser.parseDocument(query)
5153

5254
when:
53-
execution.execute(ExecutionId.generate(), MutationSchema.schema, null, null, document, null, null)
55+
execution.execute(document, MutationSchema.schema, ExecutionId.generate(), emptyExecutionInput)
5456

5557
then:
5658
0 * queryStrategy.execute(*_)
@@ -70,7 +72,7 @@ class ExecutionTest extends Specification {
7072
def document = parser.parseDocument(query)
7173

7274
when:
73-
execution.execute(ExecutionId.generate(), MutationSchema.schema, null, null, document, null, null)
75+
execution.execute(document, MutationSchema.schema, ExecutionId.generate(), emptyExecutionInput)
7476

7577
then:
7678
0 * queryStrategy.execute(*_)

0 commit comments

Comments
 (0)