Skip to content

Commit 04355ac

Browse files
andimarekandimarek
committed
refactor input object value coercion
1 parent eb3afc2 commit 04355ac

4 files changed

Lines changed: 101 additions & 34 deletions

File tree

‎build.gradle‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,18 @@ def releaseVersion = System.env.RELEASE_VERSION
4343
version = releaseVersion ? releaseVersion : getDevelopmentVersion()
4444
group = 'com.graphql-java'
4545

46+
gradle.buildFinished { buildResult ->
47+
println "*******************************"
48+
println "*"
49+
if (buildResult.failure != null) {
50+
println "* FAILURE - ${buildResult.failure}"
51+
} else {
52+
println "* SUCCESS"
53+
}
54+
println "* Version: $version"
55+
println "*"
56+
println "*******************************"
57+
}
4658

4759
repositories {
4860
mavenCentral()

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

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

3-
import static java.lang.String.format;
4-
5-
import java.util.Collections;
6-
import java.util.List;
7-
83
import graphql.ErrorType;
94
import graphql.GraphQLError;
105
import graphql.GraphQLException;
@@ -15,6 +10,11 @@
1510
import graphql.schema.GraphQLType;
1611
import graphql.schema.GraphQLTypeUtil;
1712

13+
import java.util.Collections;
14+
import java.util.List;
15+
16+
import static java.lang.String.format;
17+
1818
/**
1919
* This is thrown if a non nullable value is coerced to a null value
2020
*/
@@ -47,6 +47,12 @@ public NonNullableValueCoercedAsNullException(GraphQLInputObjectField inputTypeF
4747
inputTypeField.getName(), GraphQLTypeUtil.simplePrint(inputTypeField.getType())));
4848
}
4949

50+
public NonNullableValueCoercedAsNullException(GraphQLInputObjectField inputTypeField, List<Object> path) {
51+
super(format("Input field '%s' has coerced Null value for NonNull type '%s'",
52+
inputTypeField.getName(), GraphQLTypeUtil.simplePrint(inputTypeField.getType())));
53+
this.path = path;
54+
}
55+
5056
@Override
5157
public List<SourceLocation> getLocations() {
5258
return sourceLocations;

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

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,11 @@
2525
import graphql.schema.GraphQLType;
2626
import graphql.schema.visibility.GraphqlFieldVisibility;
2727

28+
import java.util.ArrayDeque;
2829
import java.util.ArrayList;
30+
import java.util.Arrays;
2931
import java.util.Collections;
32+
import java.util.Deque;
3033
import java.util.LinkedHashMap;
3134
import java.util.List;
3235
import java.util.Map;
@@ -58,7 +61,7 @@ public Map<String, Object> coerceVariableValues(GraphQLSchema schema, List<Varia
5861
Map<String, Object> coercedValues = new LinkedHashMap<>();
5962
for (VariableDefinition variableDefinition : variableDefinitions) {
6063
String variableName = variableDefinition.getName();
61-
List<Object> nameStack = new ArrayList<>();
64+
Deque<Object> nameStack = new ArrayDeque<>();
6265
GraphQLType variableType = TypeFromAST.getTypeFromAST(schema, variableDefinition.getType());
6366
Assert.assertTrue(variableType instanceof GraphQLInputType);
6467
// can be NullValue
@@ -153,15 +156,14 @@ private Map<String, Argument> argumentMap(List<Argument> arguments) {
153156

154157

155158
@SuppressWarnings("unchecked")
156-
private Object coerceValue(GraphqlFieldVisibility fieldVisibility, VariableDefinition variableDefinition, String inputName, GraphQLType graphQLType, Object value, List<Object> nameStack) {
159+
private Object coerceValue(GraphqlFieldVisibility fieldVisibility, VariableDefinition variableDefinition, String inputName, GraphQLType graphQLType, Object value, Deque<Object> nameStack) {
160+
nameStack.addLast(inputName);
157161
try {
158-
nameStack.add(inputName);
159-
160162
if (isNonNull(graphQLType)) {
161163
Object returnValue =
162164
coerceValue(fieldVisibility, variableDefinition, inputName, unwrapOne(graphQLType), value, nameStack);
163165
if (returnValue == null) {
164-
throw new NonNullableValueCoercedAsNullException(variableDefinition, inputName, nameStack, graphQLType);
166+
throw new NonNullableValueCoercedAsNullException(variableDefinition, inputName, Arrays.asList(nameStack.toArray()), graphQLType);
165167
}
166168
return returnValue;
167169
}
@@ -183,7 +185,7 @@ private Object coerceValue(GraphqlFieldVisibility fieldVisibility, VariableDefin
183185
throw CoercingParseValueException.newCoercingParseValueException()
184186
.message("Expected type 'Map' but was '" + value.getClass().getSimpleName() +
185187
"'. Variables for input objects must be an instance of type 'Map'.")
186-
.path(nameStack)
188+
.path(Arrays.asList(nameStack.toArray()))
187189
.build();
188190
}
189191
} else {
@@ -198,14 +200,20 @@ private Object coerceValue(GraphqlFieldVisibility fieldVisibility, VariableDefin
198200
.extensions(e.getExtensions())
199201
.cause(e.getCause())
200202
.sourceLocation(variableDefinition.getSourceLocation())
201-
.path(nameStack)
203+
.path(Arrays.asList(nameStack.toArray()))
202204
.build();
205+
} finally {
206+
nameStack.removeLast();
203207
}
204208

205209
}
206210

207-
private Object coerceValueForInputObjectType(GraphqlFieldVisibility fieldVisibility, VariableDefinition variableDefinition, GraphQLInputObjectType inputObjectType, Map<String, Object> inputMap, List<Object> nameStack) {
208-
Map<String, Object> result = new LinkedHashMap<>();
211+
private Object coerceValueForInputObjectType(GraphqlFieldVisibility fieldVisibility,
212+
VariableDefinition variableDefinition,
213+
GraphQLInputObjectType inputObjectType,
214+
Map<String, Object> inputMap,
215+
Deque<Object> nameStack) {
216+
// Map<String, Object> result = new LinkedHashMap<>();
209217
List<GraphQLInputObjectField> fields = fieldVisibility.getFieldDefinitions(inputObjectType);
210218
List<String> fieldNames = map(fields, GraphQLInputObjectField::getName);
211219
for (String inputFieldName : inputMap.keySet()) {
@@ -214,20 +222,60 @@ private Object coerceValueForInputObjectType(GraphqlFieldVisibility fieldVisibil
214222
}
215223
}
216224

217-
for (GraphQLInputObjectField inputField : fields) {
218-
if (inputMap.containsKey(inputField.getName()) || alwaysHasValue(inputField)) {
219-
// getOrDefault will return a null value if its present in the map as null
220-
// defaulting only applies if the key is missing - we want this
221-
Object inputValue = inputMap.getOrDefault(inputField.getName(), inputField.getDefaultValue());
222-
Object coerceValue = coerceValue(fieldVisibility, variableDefinition,
223-
inputField.getName(),
224-
inputField.getType(),
225-
inputValue,
226-
nameStack);
227-
result.put(inputField.getName(), coerceValue == null ? inputField.getDefaultValue() : coerceValue);
225+
Map<String, Object> coercedValues = new LinkedHashMap<>();
226+
227+
List<GraphQLInputObjectField> inputFieldTypes = fieldVisibility.getFieldDefinitions(inputObjectType);
228+
for (GraphQLInputObjectField inputFieldType : inputFieldTypes) {
229+
230+
GraphQLInputType fieldType = inputFieldType.getType();
231+
String fieldName = inputFieldType.getName();
232+
Object defaultValue = inputFieldType.getDefaultValue();
233+
boolean hasValue = inputMap.containsKey(fieldName);
234+
Object value;
235+
Object fieldValue = inputMap.getOrDefault(fieldName, null);
236+
value = fieldValue;
237+
if (!hasValue && inputFieldType.hasSetDefaultValue()) {
238+
//TODO: default value should be coerced
239+
coercedValues.put(fieldName, defaultValue);
240+
} else if (isNonNull(fieldType) && (!hasValue || value == null)) {
241+
nameStack.addLast(fieldName);
242+
throw new NonNullableValueCoercedAsNullException(inputFieldType, Arrays.asList(nameStack.toArray()));
243+
} else if (hasValue) {
244+
if (value == null) {
245+
coercedValues.put(fieldName, null);
246+
} else if (fieldValue instanceof VariableReference) {
247+
coercedValues.put(fieldName, value);
248+
} else {
249+
value = coerceValue(fieldVisibility,
250+
variableDefinition,
251+
inputFieldType.getName(),
252+
fieldType,
253+
value,
254+
nameStack);
255+
coercedValues.put(fieldName, value);
256+
}
257+
} else {
258+
// nullable type && hasValue == false && hasDefaultValue == false
259+
// meaning no value was provided for this field
228260
}
229261
}
230-
return result;
262+
return coercedValues;
263+
264+
265+
// for (GraphQLInputObjectField inputField : fields) {
266+
// if (inputMap.containsKey(inputField.getName()) || alwaysHasValue(inputField)) {
267+
// // getOrDefault will return a null value if its present in the map as null
268+
// // defaulting only applies if the key is missing - we want this
269+
// Object inputValue = inputMap.getOrDefault(inputField.getName(), inputField.getDefaultValue());
270+
// Object coerceValue = coerceValue(fieldVisibility, variableDefinition,
271+
// inputField.getName(),
272+
// inputField.getType(),
273+
// inputValue,
274+
// nameStack);
275+
// result.put(inputField.getName(), coerceValue == null ? inputField.getDefaultValue() : coerceValue);
276+
// }
277+
// }
278+
// return result;
231279
}
232280

233281
private boolean alwaysHasValue(GraphQLInputObjectField inputField) {
@@ -243,7 +291,7 @@ private Object coerceValueForEnum(GraphQLEnumType graphQLEnumType, Object value)
243291
return graphQLEnumType.parseValue(value);
244292
}
245293

246-
private List coerceValueForList(GraphqlFieldVisibility fieldVisibility, VariableDefinition variableDefinition, String inputName, GraphQLList graphQLList, Object value, List<Object> nameStack) {
294+
private List coerceValueForList(GraphqlFieldVisibility fieldVisibility, VariableDefinition variableDefinition, String inputName, GraphQLList graphQLList, Object value, Deque<Object> nameStack) {
247295
if (value instanceof Iterable) {
248296
List<Object> result = new ArrayList<>();
249297
for (Object val : (Iterable) value) {

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -362,17 +362,18 @@ class ValuesResolverTest extends Specification {
362362

363363
}
364364

365-
def "getVariableValues: input object with non-required fields and default values"() {
365+
@Unroll
366+
def "getVariableValues: input object with non-required fields and default values. #inputValue -> #outputValue"() {
366367
given:
367368

368369
def inputObjectType = newInputObject()
369370
.name("InputObject")
370371
.field(newInputObjectField()
371-
.name("intKey")
372-
.type(GraphQLInt))
372+
.name("intKey")
373+
.type(GraphQLInt))
373374
.field(newInputObjectField()
374-
.name("stringKey")
375-
.type(GraphQLString)
375+
.name("stringKey")
376+
.type(GraphQLString)
376377
.defaultValue("defaultString"))
377378
.build()
378379

@@ -388,7 +389,7 @@ class ValuesResolverTest extends Specification {
388389
where:
389390
inputValue || outputValue
390391
[intKey: 10] || [intKey: 10, stringKey: 'defaultString']
391-
[intKey: 10, stringKey: null] || [intKey: 10, stringKey: 'defaultString']
392+
[intKey: 10, stringKey: null] || [intKey: 10, stringKey: null]
392393

393394
}
394395

@@ -413,7 +414,7 @@ class ValuesResolverTest extends Specification {
413414

414415
then:
415416
def e = thrown(GraphQLException)
416-
e.path== ["variable", "intKey", "requiredField", "requiredField"]
417+
e.path == ["variable", "requiredField"]
417418

418419
where:
419420
inputValue | _

0 commit comments

Comments
 (0)