Skip to content

Commit 1b39c98

Browse files
jameskleehbbakerman
jameskleeh
authored and
bbakerman
committed
Improve error messages for arguments that fail validation (graphql-java#616)
1 parent 9d3fa8c commit 1b39c98

File tree

5 files changed

+315
-24
lines changed

5 files changed

+315
-24
lines changed

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package graphql.validation;
2+
3+
import graphql.language.Argument;
4+
import graphql.language.ObjectField;
5+
import graphql.language.Value;
6+
import graphql.schema.GraphQLEnumType;
7+
import graphql.schema.GraphQLInputObjectType;
8+
import graphql.schema.GraphQLScalarType;
9+
import graphql.schema.GraphQLType;
10+
11+
import java.util.ArrayList;
12+
import java.util.List;
13+
import java.util.Set;
14+
15+
public class ArgumentValidationUtil extends ValidationUtil {
16+
17+
private List<String> argumentNames = new ArrayList<>();
18+
private Value argumentValue;
19+
private String errorMessage;
20+
private List<Object> arguments = new ArrayList<>();
21+
22+
private String argumentName;
23+
24+
public ArgumentValidationUtil(Argument argument) {
25+
argumentName = argument.getName();
26+
argumentValue = argument.getValue();
27+
}
28+
29+
protected void handleNullError(Value value, GraphQLType type) {
30+
errorMessage = "must not be null";
31+
argumentValue = value;
32+
}
33+
34+
protected void handleScalarError(Value value, GraphQLScalarType type) {
35+
errorMessage = "is not a valid '%s'";
36+
arguments.add(type.getName());
37+
argumentValue = value;
38+
}
39+
40+
protected void handleEnumError(Value value, GraphQLEnumType type) {
41+
errorMessage = "is not a valid '%s'";
42+
arguments.add(type.getName());
43+
argumentValue = value;
44+
}
45+
46+
protected void handleNotObjectError(Value value, GraphQLInputObjectType type) {
47+
errorMessage = "must be an object type";
48+
}
49+
50+
protected void handleMissingFieldsError(Value value, GraphQLInputObjectType type, Set<String> missingFields) {
51+
errorMessage = "is missing required fields '%s'";
52+
arguments.add(missingFields);
53+
}
54+
55+
protected void handleExtraFieldError(Value value, GraphQLInputObjectType type, ObjectField objectField) {
56+
errorMessage = "contains a field not in '%s': '%s'";
57+
arguments.add(type.getName());
58+
arguments.add(objectField.getName());
59+
}
60+
61+
protected void handleFieldNotValidError(ObjectField objectField, GraphQLInputObjectType type) {
62+
argumentNames.add(0, objectField.getName());
63+
}
64+
65+
protected void handleFieldNotValidError(Value value, GraphQLType type, int index) {
66+
argumentNames.add(0, String.format("[%s]", index));
67+
}
68+
69+
public String getMessage() {
70+
StringBuilder argument = new StringBuilder(argumentName);
71+
for (String name: argumentNames) {
72+
if (name.startsWith("[")) {
73+
argument.append(name);
74+
}
75+
else {
76+
argument.append(".").append(name);
77+
}
78+
}
79+
arguments.add(0, argument.toString());
80+
arguments.add(1, argumentValue);
81+
82+
String message = "argument '%s' with value '%s'" + " " + errorMessage;
83+
84+
return String.format(message, arguments.toArray());
85+
}
86+
}

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

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66
import graphql.schema.*;
77

88
import java.util.LinkedHashMap;
9+
import java.util.List;
910
import java.util.Map;
11+
import java.util.Set;
12+
import java.util.function.Predicate;
13+
import java.util.stream.Collectors;
1014

1115
public class ValidationUtil {
1216

@@ -21,12 +25,29 @@ public TypeName getUnmodifiedType(Type type) {
2125
throw new ShouldNotHappenException();
2226
}
2327

28+
protected void handleNullError(Value value, GraphQLType type) {}
29+
30+
protected void handleScalarError(Value value, GraphQLScalarType type) {}
31+
32+
protected void handleEnumError(Value value, GraphQLEnumType type) {}
33+
34+
protected void handleNotObjectError(Value value, GraphQLInputObjectType type) {}
35+
36+
protected void handleMissingFieldsError(Value value, GraphQLInputObjectType type, Set<String> missingFields) {}
37+
38+
protected void handleExtraFieldError(Value value, GraphQLInputObjectType type, ObjectField objectField) {}
39+
40+
protected void handleFieldNotValidError(ObjectField objectField, GraphQLInputObjectType type) {}
41+
42+
protected void handleFieldNotValidError(Value value, GraphQLType type, int index) {}
43+
2444
public boolean isValidLiteralValue(Value value, GraphQLType type) {
25-
if (value == null) {
26-
return !(type instanceof GraphQLNonNull);
27-
}
28-
if (value instanceof NullValue) {
29-
return !(type instanceof GraphQLNonNull);
45+
if (value == null || value instanceof NullValue) {
46+
boolean valid = !(type instanceof GraphQLNonNull);
47+
if (!valid) {
48+
handleNullError(value, type);
49+
}
50+
return valid;
3051
}
3152
if (value instanceof VariableReference) {
3253
return true;
@@ -36,10 +57,18 @@ public boolean isValidLiteralValue(Value value, GraphQLType type) {
3657
}
3758

3859
if (type instanceof GraphQLScalarType) {
39-
return ((GraphQLScalarType) type).getCoercing().parseLiteral(value) != null;
60+
boolean valid = ((GraphQLScalarType) type).getCoercing().parseLiteral(value) != null;
61+
if (!valid) {
62+
handleScalarError(value, (GraphQLScalarType) type);
63+
}
64+
return valid;
4065
}
4166
if (type instanceof GraphQLEnumType) {
42-
return ((GraphQLEnumType) type).getCoercing().parseLiteral(value) != null;
67+
boolean valid = ((GraphQLEnumType) type).getCoercing().parseLiteral(value) != null;
68+
if (!valid) {
69+
handleEnumError(value, (GraphQLEnumType) type);
70+
}
71+
return valid;
4372
}
4473

4574
if (type instanceof GraphQLList) {
@@ -53,27 +82,40 @@ public boolean isValidLiteralValue(Value value, GraphQLType type) {
5382
}
5483

5584
private boolean isValidLiteralValue(Value value, GraphQLInputObjectType type) {
56-
if (!(value instanceof ObjectValue)) return false;
85+
if (!(value instanceof ObjectValue)) {
86+
handleNotObjectError(value, type);
87+
return false;
88+
}
5789
ObjectValue objectValue = (ObjectValue) value;
5890
Map<String, ObjectField> objectFieldMap = fieldMap(objectValue);
5991

60-
if (isFieldMissing(type, objectFieldMap)) return false;
92+
Set<String> missingFields = getMissingFields(type, objectFieldMap);
93+
if (!missingFields.isEmpty()) {
94+
handleMissingFieldsError(value, type, missingFields);
95+
return false;
96+
}
6197

6298
for (ObjectField objectField : objectValue.getObjectFields()) {
6399
GraphQLInputObjectField inputObjectField = type.getField(objectField.getName());
64-
if (inputObjectField == null) return false;
65-
if (!isValidLiteralValue(objectField.getValue(), inputObjectField.getType())) return false;
100+
if (inputObjectField == null) {
101+
handleExtraFieldError(value, type, objectField);
102+
return false;
103+
}
104+
if (!isValidLiteralValue(objectField.getValue(), inputObjectField.getType())) {
105+
handleFieldNotValidError(objectField, type);
106+
return false;
107+
}
66108

67109
}
68110
return true;
69111
}
70112

71-
private boolean isFieldMissing(GraphQLInputObjectType type, Map<String, ObjectField> objectFieldMap) {
72-
for (GraphQLInputObjectField inputObjectField : type.getFields()) {
73-
if (!objectFieldMap.containsKey(inputObjectField.getName()) &&
74-
(inputObjectField.getType() instanceof GraphQLNonNull)) return true;
75-
}
76-
return false;
113+
private Set<String> getMissingFields(GraphQLInputObjectType type, Map<String, ObjectField> objectFieldMap) {
114+
return type.getFields().stream()
115+
.filter(field -> field.getType() instanceof GraphQLNonNull)
116+
.map(GraphQLInputObjectField::getName)
117+
.filter(((Predicate<String>) objectFieldMap::containsKey).negate())
118+
.collect(Collectors.toSet());
77119
}
78120

79121
private Map<String, ObjectField> fieldMap(ObjectValue objectValue) {
@@ -87,8 +129,12 @@ private Map<String, ObjectField> fieldMap(ObjectValue objectValue) {
87129
private boolean isValidLiteralValue(Value value, GraphQLList type) {
88130
GraphQLType wrappedType = type.getWrappedType();
89131
if (value instanceof ArrayValue) {
90-
for (Value innerValue : ((ArrayValue) value).getValues()) {
91-
if (!isValidLiteralValue(innerValue, wrappedType)) return false;
132+
List<Value> values = ((ArrayValue) value).getValues();
133+
for (int i = 0; i < values.size(); i++) {
134+
if (!isValidLiteralValue(values.get(i), wrappedType)) {
135+
handleFieldNotValidError(values.get(i), wrappedType, i);
136+
return false;
137+
}
92138
}
93139
return true;
94140
} else {

‎src/main/java/graphql/validation/rules/ArgumentsOfCorrectType.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ public ArgumentsOfCorrectType(ValidationContext validationContext, ValidationErr
1515
public void checkArgument(Argument argument) {
1616
GraphQLArgument fieldArgument = getValidationContext().getArgument();
1717
if (fieldArgument == null) return;
18-
if (!getValidationUtil().isValidLiteralValue(argument.getValue(), fieldArgument.getType())) {
19-
String message = String.format("argument value %s has wrong type", argument.getValue());
20-
addError(new ValidationError(ValidationErrorType.WrongType, argument.getSourceLocation(), message));
18+
ArgumentValidationUtil validationUtil = new ArgumentValidationUtil(argument);
19+
if (!validationUtil.isValidLiteralValue(argument.getValue(), fieldArgument.getType())) {
20+
addError(new ValidationError(ValidationErrorType.WrongType, argument.getSourceLocation(), validationUtil.getMessage()));
2121
}
2222
}
2323

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ class GraphQLTest extends Specification {
274274

275275
then:
276276
result.errors.size() == 1
277-
result.errors[0].description.contains("has wrong type")
277+
result.errors[0].description == "argument 'bar' with value 'IntValue{value=12345678910}' is not a valid 'Int'"
278278
}
279279

280280
@SuppressWarnings("GroovyAssignabilityCheck")

0 commit comments

Comments
 (0)