Skip to content

Commit d6556d3

Browse files
mrdon-atlassianbbakerman
mrdon-atlassian
authored and
bbakerman
committed
More PR feedback addressed
1 parent df57417 commit d6556d3

File tree

5 files changed

+196
-33
lines changed

5 files changed

+196
-33
lines changed

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

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22

33
import graphql.ErrorType;
44
import graphql.GraphQLError;
5+
import graphql.language.Field;
56
import graphql.language.SourceLocation;
67
import graphql.schema.DataFetcher;
78

89
import java.util.ArrayList;
10+
import java.util.Collections;
911
import java.util.List;
1012
import java.util.Optional;
1113
import java.util.stream.Collectors;
1214

13-
import static java.util.Objects.requireNonNull;
15+
import static graphql.Assert.assertNotNull;
1416

1517
/**
1618
* A {@link GraphQLError} that has been changed from a {@link DataFetcher} relative error to an absolute one.
@@ -19,38 +21,14 @@ class AbsoluteGraphQLError implements GraphQLError {
1921

2022
private final List<SourceLocation> locations;
2123
private final List<Object> absolutePath;
22-
private final GraphQLError relativeError;
2324
private final String message;
2425
private final ErrorType errorType;
2526

2627
AbsoluteGraphQLError(ExecutionStrategyParameters executionStrategyParameters, GraphQLError relativeError) {
27-
requireNonNull(executionStrategyParameters);
28-
this.relativeError = requireNonNull(relativeError);
29-
this.absolutePath = Optional.ofNullable(relativeError.getPath())
30-
.map(originalPath -> {
31-
List<Object> path = new ArrayList<>();
32-
path.addAll(executionStrategyParameters.path().toList());
33-
path.addAll(relativeError.getPath());
34-
return path;
35-
})
36-
.orElse(null);
37-
38-
Optional<SourceLocation> baseLocation;
39-
if (!executionStrategyParameters.field().isEmpty()) {
40-
baseLocation = Optional.ofNullable(executionStrategyParameters.field().get(0).getSourceLocation());
41-
} else {
42-
baseLocation = Optional.empty();
43-
}
44-
45-
this.locations = Optional.ofNullable(
46-
relativeError.getLocations())
47-
.map(locations -> locations.stream()
48-
.map(l ->
49-
baseLocation
50-
.map(base -> new SourceLocation(base.getLine() + l.getLine(), base.getColumn() + l.getColumn()))
51-
.orElse(null))
52-
.collect(Collectors.toList()))
53-
.orElse(null);
28+
assertNotNull(executionStrategyParameters);
29+
assertNotNull(relativeError);
30+
this.absolutePath = createAbsolutePath(executionStrategyParameters, relativeError);
31+
this.locations = createAbsoluteLocations(relativeError, executionStrategyParameters.field());
5432
this.message = relativeError.getMessage();
5533
this.errorType = relativeError.getErrorType();
5634
}
@@ -74,4 +52,38 @@ public ErrorType getErrorType() {
7452
public List<Object> getPath() {
7553
return absolutePath;
7654
}
55+
56+
private List<Object> createAbsolutePath(ExecutionStrategyParameters executionStrategyParameters,
57+
GraphQLError relativeError) {
58+
return Optional.ofNullable(relativeError.getPath())
59+
.map(originalPath -> {
60+
List<Object> path = new ArrayList<>();
61+
path.addAll(executionStrategyParameters.path().toList());
62+
path.addAll(relativeError.getPath());
63+
return path;
64+
})
65+
.map(Collections::unmodifiableList)
66+
.orElse(null);
67+
}
68+
69+
private List<SourceLocation> createAbsoluteLocations(GraphQLError relativeError, List<Field> fields) {
70+
Optional<SourceLocation> baseLocation;
71+
if (!fields.isEmpty()) {
72+
baseLocation = Optional.ofNullable(fields.get(0).getSourceLocation());
73+
} else {
74+
baseLocation = Optional.empty();
75+
}
76+
return Optional.ofNullable(
77+
relativeError.getLocations())
78+
.map(locations -> locations.stream()
79+
.map(l ->
80+
baseLocation
81+
.map(base -> new SourceLocation(
82+
base.getLine() + l.getLine(),
83+
base.getColumn() + l.getColumn()))
84+
.orElse(null))
85+
.collect(Collectors.toList()))
86+
.map(Collections::unmodifiableList)
87+
.orElse(null);
88+
}
7789
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66

77
import java.util.List;
88

9-
import static java.util.Objects.requireNonNull;
9+
import static graphql.Assert.assertNotNull;
10+
import static java.util.Collections.unmodifiableList;
1011

1112

1213
/**
@@ -24,7 +25,7 @@ public class DataFetcherResult<T> {
2425

2526
public DataFetcherResult(T data, List<GraphQLError> errors) {
2627
this.data = data;
27-
this.errors = requireNonNull(errors);
28+
this.errors = unmodifiableList(assertNotNull(errors));
2829
}
2930

3031
/**

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ protected ExecutionStrategy(DataFetcherExceptionHandler dataFetcherExceptionHand
131131
*
132132
* @param executionContext contains the top level execution parameters
133133
* @param parameters contains the parameters holding the fields to be executed and source object
134+
*
134135
* @return an {@link ExecutionResult}
135136
* @throws NonNullableFieldWasNullException if a non null field resolves to a null value
136137
*/
@@ -147,6 +148,7 @@ protected ExecutionStrategy(DataFetcherExceptionHandler dataFetcherExceptionHand
147148
*
148149
* @param executionContext contains the top level execution parameters
149150
* @param parameters contains the parameters holding the fields to be executed and source object
151+
*
150152
* @return an {@link ExecutionResult}
151153
* @throws NonNullableFieldWasNullException if a non null field resolves to a null value
152154
*/
@@ -175,6 +177,7 @@ protected CompletableFuture<ExecutionResult> resolveField(ExecutionContext execu
175177
*
176178
* @param executionContext contains the top level execution parameters
177179
* @param parameters contains the parameters holding the fields to be executed and source object
180+
*
178181
* @return a fetched object
179182
* @throws NonNullableFieldWasNullException if a non null field resolves to a null value
180183
*/
@@ -283,6 +286,7 @@ private void handleFetchingException(ExecutionContext executionContext,
283286
* @param executionContext contains the top level execution parameters
284287
* @param parameters contains the parameters holding the fields to be executed and source object
285288
* @param fetchedValue the fetched raw value
289+
*
286290
* @return an {@link ExecutionResult}
287291
* @throws NonNullableFieldWasNullException if a non null field resolves to a null value
288292
*/
@@ -330,6 +334,7 @@ protected CompletableFuture<ExecutionResult> completeField(ExecutionContext exec
330334
*
331335
* @param executionContext contains the top level execution parameters
332336
* @param parameters contains the parameters holding the fields to be executed and source object
337+
*
333338
* @return an {@link ExecutionResult}
334339
* @throws NonNullableFieldWasNullException if a non null field resolves to a null value
335340
*/
@@ -383,6 +388,7 @@ protected CompletableFuture<ExecutionResult> completeValue(ExecutionContext exec
383388
* equals a null object and a present Optional is the underlying value.
384389
*
385390
* @param result the incoming value
391+
*
386392
* @return an un-boxed result
387393
*/
388394
protected Object unboxPossibleOptional(Object result) {
@@ -401,6 +407,7 @@ protected Object unboxPossibleOptional(Object result) {
401407
* Converts an object that is known to should be an Iterable into one
402408
*
403409
* @param result the result object
410+
*
404411
* @return an Iterable from that object
405412
* @throws java.lang.ClassCastException if its not an Iterable
406413
*/
@@ -445,6 +452,7 @@ protected GraphQLObjectType resolveType(ExecutionContext executionContext, Execu
445452
* Called to resolve a {@link GraphQLInterfaceType} into a specific {@link GraphQLObjectType} so the object can be executed in terms of that type
446453
*
447454
* @param params the params needed for type resolution
455+
*
448456
* @return a {@link GraphQLObjectType}
449457
*/
450458
protected GraphQLObjectType resolveTypeForInterface(TypeResolutionParameters params) {
@@ -460,6 +468,7 @@ protected GraphQLObjectType resolveTypeForInterface(TypeResolutionParameters par
460468
* Called to resolve a {@link GraphQLUnionType} into a specific {@link GraphQLObjectType} so the object can be executed in terms of that type
461469
*
462470
* @param params the params needed for type resolution
471+
*
463472
* @return a {@link GraphQLObjectType}
464473
*/
465474
protected GraphQLObjectType resolveTypeForUnion(TypeResolutionParameters params) {
@@ -478,6 +487,7 @@ protected GraphQLObjectType resolveTypeForUnion(TypeResolutionParameters params)
478487
* @param parameters contains the parameters holding the fields to be executed and source object
479488
* @param enumType the type of the enum
480489
* @param result the result to be coerced
490+
*
481491
* @return an {@link ExecutionResult}
482492
*/
483493
protected CompletableFuture<ExecutionResult> completeValueForEnum(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLEnumType enumType, Object result) {
@@ -499,6 +509,7 @@ protected CompletableFuture<ExecutionResult> completeValueForEnum(ExecutionConte
499509
* @param parameters contains the parameters holding the fields to be executed and source object
500510
* @param scalarType the type of the scalar
501511
* @param result the result to be coerced
512+
*
502513
* @return an {@link ExecutionResult}
503514
*/
504515
protected CompletableFuture<ExecutionResult> completeValueForScalar(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLScalarType scalarType, Object result) {
@@ -533,6 +544,7 @@ private Object handleCoercionProblem(ExecutionContext context, ExecutionStrategy
533544
* @param executionContext contains the top level execution parameters
534545
* @param parameters contains the parameters holding the fields to be executed and source object
535546
* @param iterableValues the values to complete
547+
*
536548
* @return an {@link ExecutionResult}
537549
*/
538550
protected CompletableFuture<ExecutionResult> completeValueForList(ExecutionContext executionContext, ExecutionStrategyParameters parameters, Iterable<Object> iterableValues) {
@@ -592,6 +604,7 @@ protected CompletableFuture<ExecutionResult> completeValueForList(ExecutionConte
592604
* @param executionContext contains the top level execution parameters
593605
* @param parameters contains the parameters holding the fields to be executed and source object
594606
* @param field the field to find the definition of
607+
*
595608
* @return a {@link GraphQLFieldDefinition}
596609
*/
597610
protected GraphQLFieldDefinition getFieldDef(ExecutionContext executionContext, ExecutionStrategyParameters parameters, Field field) {
@@ -605,6 +618,7 @@ protected GraphQLFieldDefinition getFieldDef(ExecutionContext executionContext,
605618
* @param schema the schema in play
606619
* @param parentType the parent type of the field
607620
* @param field the field to find the definition of
621+
*
608622
* @return a {@link GraphQLFieldDefinition}
609623
*/
610624
protected GraphQLFieldDefinition getFieldDef(GraphQLSchema schema, GraphQLObjectType parentType, Field field) {
@@ -622,6 +636,7 @@ protected GraphQLFieldDefinition getFieldDef(GraphQLSchema schema, GraphQLObject
622636
*
623637
* @param e this indicates that a null value was returned for a non null field, which needs to cause the parent field
624638
* to become null OR continue on as an exception
639+
*
625640
* @throws NonNullableFieldWasNullException if a non null field resolves to a null value
626641
*/
627642
protected void assertNonNullFieldPrecondition(NonNullableFieldWasNullException e) throws NonNullableFieldWasNullException {
@@ -660,6 +675,7 @@ protected void handleNonNullException(ExecutionContext executionContext, Complet
660675
*
661676
* @param parameters contains the parameters holding the fields to be executed and source object
662677
* @param fieldDefinition the field definition to build type info for
678+
*
663679
* @return a new type info
664680
*/
665681
protected ExecutionTypeInfo fieldTypeInfo(ExecutionStrategyParameters parameters, GraphQLFieldDefinition fieldDefinition) {

‎src/test/groovy/graphql/DataFetchingErrorGraphQLError.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import graphql.language.SourceLocation
44

55
class DataFetchingErrorGraphQLError implements GraphQLError {
66

7-
private final List<Object> path
8-
private final String message
7+
List<Object> path
8+
String message
99
List<SourceLocation> locations
1010

1111
DataFetchingErrorGraphQLError(String message) {

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

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package graphql.execution
2+
3+
import graphql.*
4+
import graphql.language.Field
5+
import graphql.language.OperationDefinition
6+
import graphql.language.SourceLocation
7+
import graphql.parser.Parser
8+
import graphql.schema.DataFetcher
9+
import graphql.schema.GraphQLSchema
10+
import spock.lang.Specification
11+
import spock.lang.Unroll
12+
13+
import static graphql.Scalars.GraphQLString
14+
import static graphql.execution.ExecutionStrategyParameters.newParameters
15+
import static graphql.execution.ExecutionTypeInfo.newTypeInfo
16+
import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition
17+
import static graphql.schema.GraphQLObjectType.newObject
18+
19+
class AbsoluteGraphQLErrorTest extends Specification {
20+
21+
def fieldDefinition = newFieldDefinition()
22+
.name("someField")
23+
.type(GraphQLString)
24+
.build()
25+
def objectType = newObject()
26+
.name("Test")
27+
.field(fieldDefinition)
28+
.build()
29+
30+
def "constructor works as expected"() {
31+
given:
32+
33+
def field = new Field("test")
34+
field.setSourceLocation(new SourceLocation(4, 5))
35+
36+
def parameters = newParameters()
37+
.typeInfo(ExecutionTypeInfo.newTypeInfo().type(objectType))
38+
.source(new Object())
39+
.fields(["fld": [new Field()]])
40+
.field([field])
41+
.path(ExecutionPath.fromList(["foo", "bar"]))
42+
.build()
43+
44+
def relativeError = new DataFetchingErrorGraphQLError("blah", ["fld"])
45+
46+
when:
47+
48+
def error = new AbsoluteGraphQLError(parameters, relativeError)
49+
50+
then:
51+
52+
error.getMessage() == "blah"
53+
error.getPath() == ["foo", "bar", "fld"]
54+
error.getLocations().size() == 1
55+
error.getLocations().get(0).getColumn() == 15
56+
error.getLocations().get(0).getLine() == 6
57+
error.getErrorType() == relativeError.getErrorType()
58+
}
59+
60+
def "constructor handles missing path as null"() {
61+
given:
62+
63+
def field = new Field("test")
64+
field.setSourceLocation(new SourceLocation(4, 5))
65+
66+
def parameters = newParameters()
67+
.typeInfo(ExecutionTypeInfo.newTypeInfo().type(objectType))
68+
.source(new Object())
69+
.fields(["fld": [new Field()]])
70+
.field([field])
71+
.path(ExecutionPath.fromList(["foo", "bar"]))
72+
.build()
73+
74+
def relativeError = new DataFetchingErrorGraphQLError("blah")
75+
76+
when:
77+
78+
def error = new AbsoluteGraphQLError(parameters, relativeError)
79+
80+
then:
81+
82+
error.getPath() == null
83+
}
84+
85+
def "constructor handles missing locations as null"() {
86+
given:
87+
88+
def field = new Field("test")
89+
90+
def parameters = newParameters()
91+
.typeInfo(ExecutionTypeInfo.newTypeInfo().type(objectType))
92+
.source(new Object())
93+
.fields(["fld": [new Field()]])
94+
.field([field])
95+
.path(ExecutionPath.fromList(["foo", "bar"]))
96+
.build()
97+
98+
def relativeError = new DataFetchingErrorGraphQLError("blah")
99+
100+
when:
101+
102+
def error = new AbsoluteGraphQLError(parameters, relativeError)
103+
104+
then:
105+
106+
error.getLocations() == null
107+
}
108+
109+
def "constructor transforms multiple source locations"() {
110+
given:
111+
112+
def field = new Field("test")
113+
field.setSourceLocation(new SourceLocation(4, 5))
114+
115+
def parameters = newParameters()
116+
.typeInfo(ExecutionTypeInfo.newTypeInfo().type(objectType))
117+
.source(new Object())
118+
.fields(["fld": [new Field()]])
119+
.field([field])
120+
.path(ExecutionPath.fromList(["foo", "bar"]))
121+
.build()
122+
123+
def relativeError = new DataFetchingErrorGraphQLError("blah", ["fld"])
124+
relativeError.locations = [new SourceLocation(1, 5), new SourceLocation(3, 6)]
125+
126+
when:
127+
128+
def error = new AbsoluteGraphQLError(parameters, relativeError)
129+
130+
then:
131+
132+
error.getLocations() == [new SourceLocation(5, 10), new SourceLocation(7, 11)]
133+
}
134+
}

0 commit comments

Comments
 (0)