Skip to content

Commit 3a392cf

Browse files
bbakermanbbakerman
committed
graphql-java#345 - mutations are optional and errors should come back if they are missing
1 parent 86132a7 commit 3a392cf

File tree

10 files changed

+116
-48
lines changed

10 files changed

+116
-48
lines changed

‎src/main/java/graphql/ErrorType.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ public enum ErrorType {
55

66
InvalidSyntax,
77
ValidationError,
8-
DataFetchingException
8+
DataFetchingException,
9+
MutationsNoSupported
910

1011
}

‎src/main/java/graphql/ExceptionWhileDataFetching.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,7 @@ public String toString() {
4242

4343
@Override
4444
public boolean equals(Object o) {
45-
if (this == o) return true;
46-
if (o == null || getClass() != o.getClass()) return false;
47-
48-
ExceptionWhileDataFetching that = (ExceptionWhileDataFetching) o;
49-
50-
return Helper.equals(this, that);
45+
return Helper.equals(this, o);
5146
}
5247

5348
@Override

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ public GraphQL(GraphQLSchema graphQLSchema, ExecutionStrategy queryStrategy, Exe
8787

8888
private GraphQL(GraphQLSchema graphQLSchema, ExecutionStrategy queryStrategy, ExecutionStrategy mutationStrategy, ExecutionIdProvider idProvider, Instrumentation instrumentation) {
8989
this.graphQLSchema = assertNotNull(graphQLSchema,"queryStrategy must be non null");
90-
this.queryStrategy = assertNotNull(queryStrategy, "queryStrategy must be non null");
90+
this.queryStrategy = queryStrategy != null ? queryStrategy : new SimpleExecutionStrategy();
91+
this.mutationStrategy = mutationStrategy != null ? mutationStrategy : new SimpleExecutionStrategy();
9192
this.idProvider = assertNotNull(idProvider, "idProvider must be non null");
92-
this.mutationStrategy = mutationStrategy;
9393
this.instrumentation = instrumentation;
9494
}
9595

@@ -143,6 +143,9 @@ public Builder executionIdProvider(ExecutionIdProvider executionIdProvider) {
143143
}
144144

145145
public GraphQL build() {
146+
assertNotNull(graphQLSchema,"queryStrategy must be non null");
147+
assertNotNull(queryExecutionStrategy, "queryStrategy must be non null");
148+
assertNotNull(idProvider, "idProvider must be non null");
146149
return new GraphQL(graphQLSchema, queryExecutionStrategy, mutationExecutionStrategy, idProvider, instrumentation);
147150
}
148151
}

‎src/main/java/graphql/GraphQLError.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,14 @@ public static int hashCode(GraphQLError dis) {
2828
return result;
2929
}
3030

31-
public static boolean equals(GraphQLError dis, GraphQLError dat) {
32-
if (dis == dat) {
31+
public static boolean equals(GraphQLError dis, Object o) {
32+
if (dis == o) {
3333
return true;
3434
}
35+
if (o == null || dis.getClass() != o.getClass()) return false;
36+
37+
GraphQLError dat = (GraphQLError) o;
38+
3539
if (dis.getMessage() != null ? !dis.getMessage().equals(dat.getMessage()) : dat.getMessage() != null)
3640
return false;
3741
if (dis.getLocations() != null ? !dis.getLocations().equals(dat.getLocations()) : dat.getLocations() != null)

‎src/main/java/graphql/InvalidSyntaxError.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,7 @@ public String toString() {
4646

4747
@Override
4848
public boolean equals(Object o) {
49-
if (this == o) return true;
50-
if (o == null || getClass() != o.getClass()) return false;
51-
52-
InvalidSyntaxError that = (InvalidSyntaxError) o;
53-
54-
return Helper.equals(this,that);
49+
return Helper.equals(this, o);
5550
}
5651

5752
@Override

‎src/main/java/graphql/MutationNotSupportedError.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package graphql;
2+
3+
4+
import graphql.language.SourceLocation;
5+
6+
import java.util.List;
7+
8+
/**
9+
* The graphql spec says that mutations are optional but it does not specify how to respond
10+
* when it is not supported. This error is returned in this case.
11+
*
12+
* http://facebook.github.io/graphql/#sec-Initial-types
13+
*/
14+
public class MutationNotSupportedError implements GraphQLError {
15+
16+
@Override
17+
public String getMessage() {
18+
return "Mutations are not supported onm this schema";
19+
}
20+
21+
@Override
22+
public List<SourceLocation> getLocations() {
23+
return null;
24+
}
25+
26+
@Override
27+
public ErrorType getErrorType() {
28+
return ErrorType.MutationsNoSupported;
29+
}
30+
31+
@Override
32+
public String toString() {
33+
return "MutationNotSupportedError";
34+
}
35+
36+
@Override
37+
public boolean equals(Object o) {
38+
return Helper.equals(this, o);
39+
}
40+
41+
@Override
42+
public int hashCode() {
43+
return Helper.hashCode(this);
44+
}
45+
}

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

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

33

44
import graphql.ExecutionResult;
5+
import graphql.ExecutionResultImpl;
56
import graphql.GraphQLException;
7+
import graphql.MutationNotSupportedError;
68
import graphql.execution.instrumentation.Instrumentation;
79
import graphql.execution.instrumentation.InstrumentationContext;
810
import graphql.execution.instrumentation.parameters.DataFetchParameters;
@@ -13,10 +15,14 @@
1315
import graphql.schema.GraphQLSchema;
1416

1517
import java.util.ArrayList;
18+
import java.util.Collections;
1619
import java.util.LinkedHashMap;
1720
import java.util.List;
1821
import java.util.Map;
1922

23+
import static graphql.language.OperationDefinition.Operation.MUTATION;
24+
import static graphql.language.OperationDefinition.Operation.QUERY;
25+
2026
public class Execution {
2127

2228
private final FieldCollector fieldCollector = new FieldCollector();
@@ -38,15 +44,15 @@ public ExecutionResult execute(ExecutionId executionId, GraphQLSchema graphQLSch
3844
return executeOperation(executionContext, root, executionContext.getOperationDefinition());
3945
}
4046

41-
private GraphQLObjectType getOperationRootType(GraphQLSchema graphQLSchema, OperationDefinition operationDefinition) {
42-
if (operationDefinition.getOperation() == OperationDefinition.Operation.MUTATION) {
47+
private GraphQLObjectType getOperationRootType(GraphQLSchema graphQLSchema, OperationDefinition.Operation operation) {
48+
if (operation == MUTATION) {
4349
return graphQLSchema.getMutationType();
4450

45-
} else if (operationDefinition.getOperation() == OperationDefinition.Operation.QUERY) {
51+
} else if (operation == QUERY) {
4652
return graphQLSchema.getQueryType();
4753

4854
} else {
49-
throw new GraphQLException();
55+
throw new GraphQLException("Unhandled case. An extra operation enum has been added without code support");
5056
}
5157
}
5258

@@ -57,13 +63,23 @@ private ExecutionResult executeOperation(
5763

5864
InstrumentationContext<ExecutionResult> dataFetchCtx = instrumentation.beginDataFetch(new DataFetchParameters(executionContext));
5965

60-
GraphQLObjectType operationRootType = getOperationRootType(executionContext.getGraphQLSchema(), operationDefinition);
66+
OperationDefinition.Operation operation = operationDefinition.getOperation();
67+
GraphQLObjectType operationRootType = getOperationRootType(executionContext.getGraphQLSchema(), operation);
68+
69+
//
70+
// do we have a mutation operation root type. The spec says if we don't then mutations are not allowed to be executed
71+
//
72+
// for the record earlier code has asserted that we have a query type in the schema since the spec says this is
73+
// ALWAYS required
74+
if (operation == MUTATION && operationRootType == null) {
75+
return new ExecutionResultImpl(Collections.singletonList(new MutationNotSupportedError()));
76+
}
6177

6278
Map<String, List<Field>> fields = new LinkedHashMap<String, List<Field>>();
6379
fieldCollector.collectFields(executionContext, operationRootType, operationDefinition.getSelectionSet(), new ArrayList<String>(), fields);
6480

6581
ExecutionResult result;
66-
if (operationDefinition.getOperation() == OperationDefinition.Operation.MUTATION) {
82+
if (operation == MUTATION) {
6783
result = mutationStrategy.execute(executionContext, operationRootType, root, fields);
6884
} else {
6985
result = queryStrategy.execute(executionContext, operationRootType, root, fields);

‎src/main/java/graphql/schema/validation/InvalidSchemaException.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,20 @@
11
package graphql.schema.validation;
22

3-
import java.util.Collection;
4-
53
import graphql.GraphQLException;
64

5+
import java.util.Collection;
6+
77
public class InvalidSchemaException extends GraphQLException {
8-
9-
private final String message;
10-
8+
119
public InvalidSchemaException(Collection<ValidationError> errors) {
10+
super(buildErrorMsg(errors));
11+
}
12+
13+
private static String buildErrorMsg(Collection<ValidationError> errors) {
1214
StringBuilder message = new StringBuilder("invalid schema:");
1315
for (ValidationError error : errors) {
1416
message.append("\n").append(error.getDescription());
1517
}
16-
this.message = message.toString();
17-
}
18-
19-
@Override
20-
public String getMessage() {
21-
return message;
18+
return message.toString();
2219
}
2320
}

‎src/main/java/graphql/validation/ValidationError.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,7 @@ public String toString() {
6464

6565
@Override
6666
public boolean equals(Object o) {
67-
if (this == o) return true;
68-
if (o == null || getClass() != o.getClass()) return false;
69-
70-
ValidationError that = (ValidationError) o;
71-
72-
return Helper.equals(this, that);
67+
return Helper.equals(this, o);
7368
}
7469

7570
@Override

‎src/test/groovy/graphql/GraphQLTest.groovy‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ class GraphQLTest extends Specification {
154154
set.add("Two")
155155

156156
def schema = GraphQLSchema.newSchema()
157-
.query(GraphQLObjectType.newObject()
158-
.name("QueryType")
159-
.field(GraphQLFieldDefinition.newFieldDefinition()
160-
.name("set")
161-
.type(new GraphQLList(GraphQLString))
162-
.dataFetcher({ set })))
163-
.build()
157+
.query(GraphQLObjectType.newObject()
158+
.name("QueryType")
159+
.field(GraphQLFieldDefinition.newFieldDefinition()
160+
.name("set")
161+
.type(new GraphQLList(GraphQLString))
162+
.dataFetcher({ set })))
163+
.build()
164164

165165
when:
166166
def data = GraphQL.newGraphQL(schema).build().execute("query { set }").data
@@ -203,7 +203,7 @@ class GraphQLTest extends Specification {
203203
.name("RootQueryType")
204204
.field(newFieldDefinition().name("name").type(GraphQLString))
205205
)
206-
.build()
206+
.build()
207207

208208
def query = """
209209
query Query1 { name }
@@ -216,4 +216,21 @@ class GraphQLTest extends Specification {
216216
then:
217217
thrown(GraphQLException)
218218
}
219+
220+
def "null mutation type does not throw an npe re: #345 but returns and error"() {
221+
given:
222+
223+
GraphQLSchema schema = newSchema().query(
224+
newObject()
225+
.name("Query")
226+
)
227+
.build()
228+
229+
when:
230+
def result = new GraphQL(schema).execute("mutation { doesNotExist }");
231+
232+
then:
233+
result.errors.size() == 1
234+
result.errors[0].class == MutationNotSupportedError
235+
}
219236
}

0 commit comments

Comments
 (0)