Skip to content

Commit fcbce54

Browse files
authored
#427 TDD driven null support (#452)
* #427 added null support but have not added optional arguments yet * #427 added spec tests and values resolver support * #427 more tests and a specific exception for coerced null values * #427 added null support but have not added optional arguments yet * #427 added spec tests and values resolver support * #427 more tests and a specific exception for coerced null values * add null value parser test * cleanup: remove dead code * #427 - more tests for null variables values * #427 - extra test for This unordered map should not contain any entries with names not defined by a field of this input object type, otherwise an error should be thrown.
1 parent b8fdebb commit fcbce54

18 files changed

+635
-25
lines changed

src/main/antlr/Graphql.g4

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ IntValue |
6666
FloatValue |
6767
StringValue |
6868
BooleanValue |
69+
NullValue |
6970
enumValue |
7071
arrayValue |
7172
objectValue;
@@ -76,6 +77,7 @@ IntValue |
7677
FloatValue |
7778
StringValue |
7879
BooleanValue |
80+
NullValue |
7981
enumValue |
8082
arrayValueWithVariable |
8183
objectValueWithVariable;
@@ -176,6 +178,8 @@ directiveLocations '|' directiveLocation
176178

177179
BooleanValue: 'true' | 'false';
178180

181+
NullValue: 'null';
182+
179183
FRAGMENT: 'fragment';
180184
QUERY: 'query';
181185
MUTATION: 'mutation';

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import graphql.execution.instrumentation.parameters.FieldFetchParameters;
1212
import graphql.execution.instrumentation.parameters.FieldParameters;
1313
import graphql.language.Field;
14+
import graphql.schema.DataFetcher;
1415
import graphql.schema.DataFetchingEnvironment;
1516
import graphql.schema.DataFetchingEnvironmentImpl;
1617
import graphql.schema.DataFetchingFieldSelectionSet;
@@ -96,7 +97,8 @@ protected ExecutionResult resolveField(ExecutionContext executionContext, Execut
9697
InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(new FieldFetchParameters(executionContext, fieldDef, environment));
9798
Object resolvedValue = null;
9899
try {
99-
resolvedValue = fieldDef.getDataFetcher().get(environment);
100+
DataFetcher dataFetcher = fieldDef.getDataFetcher();
101+
resolvedValue = dataFetcher.get(environment);
100102

101103
fetchCtx.onEnd(resolvedValue);
102104
} catch (Exception e) {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package graphql.execution;
2+
3+
import graphql.GraphQLException;
4+
import graphql.schema.GraphQLType;
5+
import graphql.schema.GraphQLTypeUtil;
6+
7+
/**
8+
* https://facebook.github.io/graphql/#sec-Input-Objects
9+
*
10+
* - This unordered map should not contain any entries with names not defined by a field of this input object type, otherwise an error should be thrown.
11+
*/
12+
public class InputMapDefinesTooManyFieldsException extends GraphQLException {
13+
14+
public InputMapDefinesTooManyFieldsException(GraphQLType graphQLType, String fieldName) {
15+
super(String.format("The variables input contains a field name '%s' that is not defined for input object type '%s' ", GraphQLTypeUtil.getUnwrappedTypeName(graphQLType), fieldName));
16+
}
17+
18+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package graphql.execution;
2+
3+
import graphql.GraphQLException;
4+
import graphql.schema.GraphQLType;
5+
import graphql.schema.GraphQLTypeUtil;
6+
7+
/**
8+
* This is thrown if a non nullable value is coerced to a null value
9+
*/
10+
public class NonNullableValueCoercedAsNullException extends GraphQLException {
11+
12+
public NonNullableValueCoercedAsNullException(GraphQLType graphQLType) {
13+
super(String.format("Null value for NonNull type '%s", GraphQLTypeUtil.getUnwrappedTypeName(graphQLType)));
14+
}
15+
16+
}

src/main/java/graphql/execution/ValuesResolver.java

Lines changed: 86 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,44 @@
22

33

44
import graphql.GraphQLException;
5-
import graphql.language.*;
6-
import graphql.schema.*;
5+
import graphql.language.Argument;
6+
import graphql.language.ArrayValue;
7+
import graphql.language.NullValue;
8+
import graphql.language.ObjectField;
9+
import graphql.language.ObjectValue;
10+
import graphql.language.Value;
11+
import graphql.language.VariableDefinition;
12+
import graphql.language.VariableReference;
13+
import graphql.schema.GraphQLArgument;
14+
import graphql.schema.GraphQLEnumType;
15+
import graphql.schema.GraphQLInputObjectField;
16+
import graphql.schema.GraphQLInputObjectType;
17+
import graphql.schema.GraphQLList;
18+
import graphql.schema.GraphQLNonNull;
19+
import graphql.schema.GraphQLScalarType;
20+
import graphql.schema.GraphQLSchema;
21+
import graphql.schema.GraphQLType;
722

8-
import java.util.*;
23+
import java.util.ArrayList;
24+
import java.util.Collections;
25+
import java.util.LinkedHashMap;
26+
import java.util.List;
27+
import java.util.Map;
28+
import java.util.stream.Collectors;
929

1030
public class ValuesResolver {
1131

1232

13-
public Map<String, Object> getVariableValues(GraphQLSchema schema, List<VariableDefinition> variableDefinitions, Map<String, Object> inputs) {
33+
public Map<String, Object> getVariableValues(GraphQLSchema schema, List<VariableDefinition> variableDefinitions, Map<String, Object> args) {
1434
Map<String, Object> result = new LinkedHashMap<>();
1535
for (VariableDefinition variableDefinition : variableDefinitions) {
16-
result.put(variableDefinition.getName(), getVariableValue(schema, variableDefinition, inputs.get(variableDefinition.getName())));
36+
String varName = variableDefinition.getName();
37+
// we transfer the variable as field arguments if its present as value
38+
if (args.containsKey(varName)) {
39+
Object arg = args.get(varName);
40+
Object variableValue = getVariableValue(schema, variableDefinition, arg);
41+
result.put(varName, variableValue);
42+
}
1743
}
1844
return result;
1945
}
@@ -23,14 +49,19 @@ public Map<String, Object> getArgumentValues(List<GraphQLArgument> argumentTypes
2349
Map<String, Object> result = new LinkedHashMap<>();
2450
Map<String, Argument> argumentMap = argumentMap(arguments);
2551
for (GraphQLArgument fieldArgument : argumentTypes) {
26-
Argument argument = argumentMap.get(fieldArgument.getName());
52+
String argName = fieldArgument.getName();
53+
Argument argument = argumentMap.get(argName);
2754
Object value;
2855
if (argument != null) {
2956
value = coerceValueAst(fieldArgument.getType(), argument.getValue(), variables);
3057
} else {
3158
value = fieldArgument.getDefaultValue();
3259
}
33-
result.put(fieldArgument.getName(), value);
60+
// only put an arg into the result IF they specified a variable at all or
61+
// the default value ended up being something non null
62+
if (argumentMap.containsKey(argName) || value != null) {
63+
result.put(argName, value);
64+
}
3465
}
3566
return result;
3667
}
@@ -48,6 +79,7 @@ private Map<String, Argument> argumentMap(List<Argument> arguments) {
4879
private Object getVariableValue(GraphQLSchema schema, VariableDefinition variableDefinition, Object inputValue) {
4980
GraphQLType type = TypeFromAST.getTypeFromAST(schema, variableDefinition.getType());
5081

82+
//noinspection ConstantConditions
5183
if (!isValid(type, inputValue)) {
5284
throw new GraphQLException("Invalid value for type");
5385
}
@@ -67,7 +99,7 @@ private Object coerceValue(GraphQLType graphQLType, Object value) {
6799
if (graphQLType instanceof GraphQLNonNull) {
68100
Object returnValue = coerceValue(((GraphQLNonNull) graphQLType).getWrappedType(), value);
69101
if (returnValue == null) {
70-
throw new GraphQLException("Null value for NonNull type " + graphQLType);
102+
throw new NonNullableValueCoercedAsNullException(graphQLType);
71103
}
72104
return returnValue;
73105
}
@@ -81,6 +113,7 @@ private Object coerceValue(GraphQLType graphQLType, Object value) {
81113
} else if (graphQLType instanceof GraphQLList) {
82114
return coerceValueForList((GraphQLList) graphQLType, value);
83115
} else if (graphQLType instanceof GraphQLInputObjectType && value instanceof Map) {
116+
//noinspection unchecked
84117
return coerceValueForInputObjectType((GraphQLInputObjectType) graphQLType, (Map<String, Object>) value);
85118
} else if (graphQLType instanceof GraphQLInputObjectType) {
86119
return value;
@@ -91,7 +124,15 @@ private Object coerceValue(GraphQLType graphQLType, Object value) {
91124

92125
private Object coerceValueForInputObjectType(GraphQLInputObjectType inputObjectType, Map<String, Object> input) {
93126
Map<String, Object> result = new LinkedHashMap<>();
94-
for (GraphQLInputObjectField inputField : inputObjectType.getFields()) {
127+
List<GraphQLInputObjectField> fields = inputObjectType.getFields();
128+
List<String> fieldNames = fields.stream().map(GraphQLInputObjectField::getName).collect(Collectors.toList());
129+
for (String inputFieldName : input.keySet()) {
130+
if (!fieldNames.contains(inputFieldName)) {
131+
throw new InputMapDefinesTooManyFieldsException(inputObjectType, inputFieldName);
132+
}
133+
}
134+
135+
for (GraphQLInputObjectField inputField : fields) {
95136
if (input.containsKey(inputField.getName()) || alwaysHasValue(inputField)) {
96137
Object value = coerceValue(inputField.getType(), input.get(inputField.getName()));
97138
result.put(inputField.getName(), value == null ? inputField.getDefaultValue() : value);
@@ -129,6 +170,9 @@ private Object coerceValueAst(GraphQLType type, Value inputValue, Map<String, Ob
129170
if (inputValue instanceof VariableReference) {
130171
return variables.get(((VariableReference) inputValue).getName());
131172
}
173+
if (inputValue instanceof NullValue) {
174+
return null;
175+
}
132176
if (type instanceof GraphQLScalarType) {
133177
return ((GraphQLScalarType) type).getCoercing().parseLiteral(inputValue);
134178
}
@@ -167,22 +211,48 @@ private Object coerceValueAstForInputObject(GraphQLInputObjectType type, ObjectV
167211

168212
for (GraphQLInputObjectField inputTypeField : type.getFields()) {
169213
if (inputValueFieldsByName.containsKey(inputTypeField.getName())) {
214+
boolean putObjectInMap = true;
215+
170216
ObjectField field = inputValueFieldsByName.get(inputTypeField.getName());
171-
Object fieldValue = coerceValueAst(inputTypeField.getType(), field.getValue(), variables);
172-
if (fieldValue == null) {
173-
fieldValue = inputTypeField.getDefaultValue();
217+
Value fieldInputValue = field.getValue();
218+
219+
Object fieldObject = null;
220+
if (fieldInputValue instanceof VariableReference) {
221+
String varName = ((VariableReference) fieldInputValue).getName();
222+
if (!variables.containsKey(varName)) {
223+
putObjectInMap = false;
224+
} else {
225+
fieldObject = variables.get(varName);
226+
}
227+
} else {
228+
fieldObject = coerceValueAst(inputTypeField.getType(), fieldInputValue, variables);
229+
}
230+
231+
if (fieldObject == null) {
232+
if (!field.getValue().isEqualTo(NullValue.Null)) {
233+
fieldObject = inputTypeField.getDefaultValue();
234+
}
235+
}
236+
if (putObjectInMap) {
237+
result.put(field.getName(), fieldObject);
238+
} else {
239+
assertNonNullInputField(inputTypeField);
174240
}
175-
result.put(field.getName(), fieldValue);
176241
} else if (inputTypeField.getDefaultValue() != null) {
177242
result.put(inputTypeField.getName(), inputTypeField.getDefaultValue());
178-
} else if (inputTypeField.getType() instanceof GraphQLNonNull) {
179-
// Possibly overkill; an object literal with a missing non null field shouldn't pass validation
180-
throw new GraphQLException("Null value for NonNull type " + inputTypeField.getType());
243+
} else {
244+
assertNonNullInputField(inputTypeField);
181245
}
182246
}
183247
return result;
184248
}
185249

250+
private void assertNonNullInputField(GraphQLInputObjectField inputTypeField) {
251+
if (inputTypeField.getType() instanceof GraphQLNonNull) {
252+
throw new NonNullableValueCoercedAsNullException(inputTypeField.getType());
253+
}
254+
}
255+
186256
private Map<String, ObjectField> mapObjectValueFieldsByName(ObjectValue inputValue) {
187257
Map<String, ObjectField> inputValueFieldsByName = new LinkedHashMap<>();
188258
for (ObjectField objectField : inputValue.getObjectFields()) {

src/main/java/graphql/language/AstPrinter.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class AstPrinter {
2626
printers.put(Argument.class, argument());
2727
printers.put(ArrayValue.class, value());
2828
printers.put(BooleanValue.class, value());
29+
printers.put(NullValue.class, value());
2930
printers.put(Directive.class, directive());
3031
printers.put(DirectiveDefinition.class, directiveDefinition());
3132
printers.put(DirectiveLocation.class, directiveLocation());
@@ -399,6 +400,8 @@ static private String value(Value value) {
399400
return valueOf(((EnumValue) value).getName());
400401
} else if (value instanceof BooleanValue) {
401402
return valueOf(((BooleanValue) value).isValue());
403+
} else if (value instanceof NullValue) {
404+
return "null";
402405
} else if (value instanceof ArrayValue) {
403406
return "[" + join(((ArrayValue) value).getValues(), ", ") + "]";
404407
} else if (value instanceof ObjectValue) {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package graphql.language;
2+
3+
4+
import java.util.Collections;
5+
import java.util.List;
6+
7+
public class NullValue extends AbstractNode implements Value {
8+
9+
public static NullValue Null = new NullValue();
10+
11+
private NullValue() {
12+
}
13+
14+
@Override
15+
public List<Node> getChildren() {
16+
return Collections.emptyList();
17+
}
18+
19+
@Override
20+
public boolean isEqualTo(Node o) {
21+
if (this == o) return true;
22+
if (o == null || getClass() != o.getClass()) return false;
23+
24+
return true;
25+
26+
}
27+
28+
@Override
29+
public String toString() {
30+
return "NullValue{" +
31+
'}';
32+
}
33+
}

src/main/java/graphql/parser/GraphqlAntlrToLanguage.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@
5858
import java.util.Deque;
5959
import java.util.List;
6060

61+
import static graphql.language.NullValue.Null;
62+
6163
@Internal
64+
6265
public class GraphqlAntlrToLanguage extends GraphqlBaseVisitor<Void> {
6366

6467
private final CommonTokenStream tokens;
@@ -668,6 +671,9 @@ private Value getValue(GraphqlParser.ValueWithVariableContext ctx) {
668671
BooleanValue booleanValue = new BooleanValue(Boolean.parseBoolean(ctx.BooleanValue().getText()));
669672
newNode(booleanValue, ctx);
670673
return booleanValue;
674+
} else if (ctx.NullValue() != null) {
675+
newNode(Null, ctx);
676+
return Null;
671677
} else if (ctx.StringValue() != null) {
672678
StringValue stringValue = new StringValue(parseString(ctx.StringValue().getText()));
673679
newNode(stringValue, ctx);
@@ -713,6 +719,9 @@ private Value getValue(GraphqlParser.ValueContext ctx) {
713719
BooleanValue booleanValue = new BooleanValue(Boolean.parseBoolean(ctx.BooleanValue().getText()));
714720
newNode(booleanValue, ctx);
715721
return booleanValue;
722+
} else if (ctx.NullValue() != null) {
723+
newNode(Null, ctx);
724+
return Null;
716725
} else if (ctx.StringValue() != null) {
717726
StringValue stringValue = new StringValue(parseString(ctx.StringValue().getText()));
718727
newNode(stringValue, ctx);

src/main/java/graphql/schema/idl/SchemaGenerator.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import graphql.language.IntValue;
1414
import graphql.language.InterfaceTypeDefinition;
1515
import graphql.language.Node;
16+
import graphql.language.NullValue;
1617
import graphql.language.ObjectTypeDefinition;
1718
import graphql.language.ObjectValue;
1819
import graphql.language.OperationTypeDefinition;
@@ -475,6 +476,8 @@ private Object buildValue(Value value) {
475476
result = arrayValue.getValues().stream().map(this::buildValue).toArray();
476477
} else if (value instanceof ObjectValue) {
477478
result = buildObjectValue((ObjectValue) value);
479+
} else if (value instanceof NullValue) {
480+
result = null;
478481
}
479482
return result;
480483

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ public boolean isValidLiteralValue(Value value, GraphQLType type) {
2525
if (value == null) {
2626
return !(type instanceof GraphQLNonNull);
2727
}
28+
if (value instanceof NullValue) {
29+
return !(type instanceof GraphQLNonNull);
30+
}
2831
if (value instanceof VariableReference) {
2932
return true;
3033
}

0 commit comments

Comments
 (0)