Skip to content

Commit 080c6a3

Browse files
andimarekandimarek
authored
Merge pull request #2315 from graphql-java/normalized-fields-lists
fix handling of NormalizedInputValue in Lists
2 parents d3ac10b + ad32cd1 commit 080c6a3

5 files changed

Lines changed: 130 additions & 57 deletions

File tree

‎build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ dependencies {
8484
implementation 'com.google.guava:guava:30.0-jre'
8585
testImplementation group: 'junit', name: 'junit', version: '4.12'
8686
testImplementation 'org.spockframework:spock-core:1.3-groovy-2.5'
87-
testImplementation 'org.codehaus.groovy:groovy-all:2.5.13'
87+
testImplementation('org.codehaus.groovy:groovy:2.5.13')
8888
testImplementation 'cglib:cglib-nodep:3.3.0'
8989
testImplementation 'org.objenesis:objenesis:2.1'
9090
testImplementation 'com.google.code.gson:gson:2.8.0'

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

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ private Map<String, Object> coerceVariableValuesImpl(GraphQLSchema schema,
120120
boolean hasValue = rawVariables.containsKey(variableName);
121121
Object value = rawVariables.get(variableName);
122122
if (!hasValue && defaultValue != null) {
123-
Object coercedDefaultValue = coerceValueAst(fieldVisibility, variableType, defaultValue, Collections.emptyMap(), null, valueMode);
123+
Object coercedDefaultValue = coerceAstValue(fieldVisibility, variableType, defaultValue, Collections.emptyMap(), null, valueMode, false);
124124
coercedValues.put(variableName, newValue(coercedDefaultValue, variableType, valueMode));
125125
} else if (isNonNull(variableType) && (!hasValue || value == null)) {
126126
throw new NonNullableValueCoercedAsNullException(variableDefinition, variableType);
@@ -180,7 +180,7 @@ private Map<String, Object> getArgumentValuesImpl(GraphQLCodeRegistry codeRegist
180180
} else if (argumentValue instanceof VariableReference) {
181181
coercedValues.put(argumentName, value);
182182
} else {
183-
value = coerceValueAst(codeRegistry.getFieldVisibility(), argumentType, argument.getValue(), coercedVariables, normalizedVariables, valueMode);
183+
value = coerceAstValue(codeRegistry.getFieldVisibility(), argumentType, argument.getValue(), coercedVariables, normalizedVariables, valueMode, false);
184184
coercedValues.put(argumentName, newValue(value, argumentType, valueMode));
185185
}
186186
} else {
@@ -364,14 +364,24 @@ private List coerceValueForList(GraphqlFieldVisibility fieldVisibility,
364364
}
365365
}
366366

367-
private Object coerceValueAst(GraphqlFieldVisibility fieldVisibility,
368-
GraphQLType type,
369-
Value inputValue,
370-
Map<String, Object> coercedVariables,
371-
@Nullable Map<String, NormalizedInputValue> normalizedVariables,
372-
ValueMode valueMode) {
367+
public Object coerceAstValue(GraphqlFieldVisibility fieldVisibility,
368+
GraphQLType type,
369+
Value inputValue,
370+
Map<String, Object> coercedVariables,
371+
@Nullable Map<String, NormalizedInputValue> normalizedVariables,
372+
ValueMode valueMode,
373+
boolean unwrappingList) {
374+
373375
if (inputValue instanceof VariableReference) {
374-
return coercedVariables.get(((VariableReference) inputValue).getName());
376+
Map<String, Object> variables = getVariables(coercedVariables, normalizedVariables, valueMode);
377+
Object variableValue = variables.get(((VariableReference) inputValue).getName());
378+
// this is a special case when we have a normalized variable inside a List:
379+
// we just need to the value here, because the whole list itself is already a NormalizedInputValue
380+
if (unwrappingList && variableValue instanceof NormalizedInputValue) {
381+
return ((NormalizedInputValue) variableValue).getValue();
382+
} else {
383+
return variableValue;
384+
}
375385
}
376386
if (inputValue instanceof NullValue) {
377387
return null;
@@ -380,16 +390,16 @@ private Object coerceValueAst(GraphqlFieldVisibility fieldVisibility,
380390
return parseLiteral(inputValue, ((GraphQLScalarType) type).getCoercing(), coercedVariables);
381391
}
382392
if (isNonNull(type)) {
383-
return coerceValueAst(fieldVisibility, unwrapOne(type), inputValue, coercedVariables, normalizedVariables, valueMode);
393+
return coerceAstValue(fieldVisibility, unwrapOne(type), inputValue, coercedVariables, normalizedVariables, valueMode, unwrappingList);
384394
}
385395
if (type instanceof GraphQLInputObjectType) {
386-
return coerceValueAstForInputObject(fieldVisibility, (GraphQLInputObjectType) type, (ObjectValue) inputValue, coercedVariables, normalizedVariables, valueMode);
396+
return coerceAstValueForInputObject(fieldVisibility, (GraphQLInputObjectType) type, (ObjectValue) inputValue, coercedVariables, normalizedVariables, valueMode);
387397
}
388398
if (type instanceof GraphQLEnumType) {
389399
return ((GraphQLEnumType) type).parseLiteral(inputValue);
390400
}
391401
if (isList(type)) {
392-
return coerceValueAstForList(fieldVisibility, (GraphQLList) type, inputValue, coercedVariables, normalizedVariables, valueMode);
402+
return coerceAstValueAstForList(fieldVisibility, (GraphQLList) type, inputValue, coercedVariables, normalizedVariables, valueMode);
393403
}
394404
return null;
395405
}
@@ -399,30 +409,30 @@ private Object parseLiteral(Value inputValue, Coercing coercing, Map<String, Obj
399409
return coercing.parseLiteral(inputValue, variables);
400410
}
401411

402-
private Object coerceValueAstForList(GraphqlFieldVisibility fieldVisibility,
403-
GraphQLList graphQLList,
404-
Value value,
405-
Map<String, Object> coercedVariables,
406-
@Nullable Map<String, NormalizedInputValue> normalizedVariables,
407-
ValueMode valueMode) {
412+
private Object coerceAstValueAstForList(GraphqlFieldVisibility fieldVisibility,
413+
GraphQLList graphQLList,
414+
Value value,
415+
Map<String, Object> coercedVariables,
416+
@Nullable Map<String, NormalizedInputValue> normalizedVariables,
417+
ValueMode valueMode) {
408418
if (value instanceof ArrayValue) {
409419
ArrayValue arrayValue = (ArrayValue) value;
410420
List<Object> result = new ArrayList<>();
411421
for (Value singleValue : arrayValue.getValues()) {
412-
result.add(coerceValueAst(fieldVisibility, graphQLList.getWrappedType(), singleValue, coercedVariables, normalizedVariables, valueMode));
422+
result.add(coerceAstValue(fieldVisibility, graphQLList.getWrappedType(), singleValue, coercedVariables, normalizedVariables, valueMode, true));
413423
}
414424
return result;
415425
} else {
416-
return Collections.singletonList(coerceValueAst(fieldVisibility,
426+
return Collections.singletonList(coerceAstValue(fieldVisibility,
417427
graphQLList.getWrappedType(),
418428
value,
419429
coercedVariables,
420430
normalizedVariables,
421-
valueMode));
431+
valueMode, true));
422432
}
423433
}
424434

425-
private Object coerceValueAstForInputObject(GraphqlFieldVisibility fieldVisibility,
435+
private Object coerceAstValueForInputObject(GraphqlFieldVisibility fieldVisibility,
426436
GraphQLInputObjectType type,
427437
ObjectValue inputValue,
428438
Map<String, Object> coercedVariables,
@@ -462,7 +472,7 @@ private Object coerceValueAstForInputObject(GraphqlFieldVisibility fieldVisibili
462472
} else if (fieldValue instanceof VariableReference) {
463473
coercedValues.put(fieldName, value);
464474
} else {
465-
value = coerceValueAst(fieldVisibility, fieldType, fieldValue, coercedVariables, normalizedVariables, valueMode);
475+
value = coerceAstValue(fieldVisibility, fieldType, fieldValue, coercedVariables, normalizedVariables, valueMode, true);
466476
coercedValues.put(fieldName, newValue(value, fieldType, valueMode));
467477
}
468478
} else {

‎src/main/java/graphql/normalized/NormalizedInputValue.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,24 @@ public Object getValue() {
3535
return value;
3636
}
3737

38+
39+
public boolean isList() {
40+
return typeName.startsWith("[");
41+
}
42+
43+
public String getUnwrappedTypeName() {
44+
String result = unwrapNonNull(typeName);
45+
while (result.startsWith("[")) {
46+
result = result.substring(1, result.length() - 2);
47+
result = unwrapNonNull(result);
48+
}
49+
return result;
50+
}
51+
52+
private String unwrapNonNull(String string) {
53+
return string.endsWith("!") ? string.substring(0, string.length() - 2) : string;
54+
}
55+
3856
@Override
3957
public boolean equals(Object o) {
4058
if (this == o) {

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

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class ValuesResolverTest extends Specification {
4141
def "getVariableValues: simple variable input #inputValue"() {
4242
given:
4343
def schema = TestUtil.schemaWithInputType(inputType)
44-
VariableDefinition variableDefinition = new VariableDefinition("variable", variableType,null)
44+
VariableDefinition variableDefinition = new VariableDefinition("variable", variableType, null)
4545
when:
4646
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: inputValue])
4747
then:
@@ -167,20 +167,20 @@ class ValuesResolverTest extends Specification {
167167
def subObjectType = newInputObject()
168168
.name("SubType")
169169
.field(newInputObjectField()
170-
.name("subKey")
171-
.type(GraphQLBoolean))
170+
.name("subKey")
171+
.type(GraphQLBoolean))
172172
.build()
173173
def inputObjectType = newInputObject()
174174
.name("inputObject")
175175
.field(newInputObjectField()
176-
.name("intKey")
177-
.type(GraphQLInt))
176+
.name("intKey")
177+
.type(GraphQLInt))
178178
.field(newInputObjectField()
179-
.name("stringKey")
180-
.type(GraphQLString))
179+
.name("stringKey")
180+
.type(GraphQLString))
181181
.field(newInputObjectField()
182-
.name("subObject")
183-
.type(subObjectType))
182+
.name("subObject")
183+
.type(subObjectType))
184184
.build()
185185
def fieldArgument = new GraphQLArgument("arg", inputObjectType)
186186

@@ -220,15 +220,15 @@ class ValuesResolverTest extends Specification {
220220
def inputObjectType = newInputObject()
221221
.name("inputObject")
222222
.field(newInputObjectField()
223-
.name("intKey")
224-
.type(nonNull(GraphQLInt))
225-
.defaultValue(3)
226-
.build())
223+
.name("intKey")
224+
.type(nonNull(GraphQLInt))
225+
.defaultValue(3)
226+
.build())
227227
.field(newInputObjectField()
228-
.name("stringKey")
229-
.type(GraphQLString)
230-
.defaultValue("defaultString")
231-
.build())
228+
.name("stringKey")
229+
.type(GraphQLString)
230+
.defaultValue("defaultString")
231+
.build())
232232
.build()
233233
def fieldArgument = new GraphQLArgument("arg", inputObjectType)
234234

@@ -262,9 +262,9 @@ class ValuesResolverTest extends Specification {
262262
def inputObjectType = newInputObject()
263263
.name("inputObject")
264264
.field(newInputObjectField()
265-
.name("intKey")
266-
.type(nonNull(GraphQLInt))
267-
.build())
265+
.name("intKey")
266+
.type(nonNull(GraphQLInt))
267+
.build())
268268
.build()
269269
def fieldArgument = new GraphQLArgument("arg", inputObjectType)
270270

@@ -374,7 +374,7 @@ class ValuesResolverTest extends Specification {
374374
.field(newInputObjectField()
375375
.name("stringKey")
376376
.type(GraphQLString)
377-
.defaultValue("defaultString"))
377+
.defaultValue("defaultString"))
378378
.build()
379379

380380
def schema = TestUtil.schemaWithInputType(inputObjectType)
@@ -399,11 +399,11 @@ class ValuesResolverTest extends Specification {
399399
def inputObjectType = newInputObject()
400400
.name("InputObject")
401401
.field(newInputObjectField()
402-
.name("intKey")
403-
.type(GraphQLInt))
402+
.name("intKey")
403+
.type(GraphQLInt))
404404
.field(newInputObjectField()
405-
.name("requiredField")
406-
.type(nonNull(GraphQLString)))
405+
.name("requiredField")
406+
.type(nonNull(GraphQLString)))
407407
.build()
408408

409409
def schema = TestUtil.schemaWithInputType(inputObjectType)

‎src/test/groovy/graphql/normalized/NormalizedQueryTreeFactoryTest.groovy‎

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,9 +1052,9 @@ schema {
10521052
'Friend.name: String (conditional: false)']
10531053
}
10541054

1055-
private void assertValidQuery(GraphQLSchema graphQLSchema, String query) {
1055+
private void assertValidQuery(GraphQLSchema graphQLSchema, String query, Map variables = [:]) {
10561056
GraphQL graphQL = GraphQL.newGraphQL(graphQLSchema).build();
1057-
assert graphQL.execute(query).errors.size() == 0
1057+
assert graphQL.execute(query, null, variables).errors.size() == 0
10581058
}
10591059

10601060
def "normalized arguments"() {
@@ -1130,17 +1130,18 @@ schema {
11301130
GraphQLSchema graphQLSchema = TestUtil.schema(schema)
11311131

11321132
String query = '''
1133-
query($var1: [Input1]){
1134-
search(arg1:["1","2"], arg2: [[{foo: "foo1", input2: {bar: 123}},{foo: "foo2", input2: {bar: 456}}]], arg3: $var1)
1133+
query($var1: [Input1], $var2: ID!){
1134+
search(arg1:["1",$var2], arg2: [[{foo: "foo1", input2: {bar: 123}},{foo: "foo2", input2: {bar: 456}}]], arg3: $var1)
11351135
}
11361136
'''
11371137

1138-
assertValidQuery(graphQLSchema, query)
1139-
Document document = TestUtil.parseQuery(query)
1140-
NormalizedQueryTreeFactory dependencyGraph = new NormalizedQueryTreeFactory();
11411138
def variables = [
1142-
var1: [[foo: "foo3", input2: [bar: 789]]]
1139+
var1: [[foo: "foo3", input2: [bar: 789]]],
1140+
var2: "2",
11431141
]
1142+
assertValidQuery(graphQLSchema, query, variables)
1143+
Document document = TestUtil.parseQuery(query)
1144+
NormalizedQueryTreeFactory dependencyGraph = new NormalizedQueryTreeFactory();
11441145
when:
11451146
def tree = dependencyGraph.createNormalizedQueryWithRawVariables(graphQLSchema, document, null, variables)
11461147
def topLevelField = tree.getTopLevelFields().get(0)
@@ -1165,4 +1166,48 @@ schema {
11651166

11661167
}
11671168

1169+
def "normalized arguments with lists 2"() {
1170+
given:
1171+
String schema = """
1172+
type Query{
1173+
search(arg1:[[Input1]] ,arg2:[[ID!]!]): Boolean
1174+
}
1175+
input Input1 {
1176+
foo: String
1177+
input2: Input2
1178+
}
1179+
input Input2 {
1180+
bar: Int
1181+
}
1182+
"""
1183+
GraphQLSchema graphQLSchema = TestUtil.schema(schema)
1184+
1185+
String query = '''
1186+
query($var1: [Input1], $var2: [ID!]!){
1187+
search(arg1: [$var1],arg2:[["1"],$var2] )
1188+
}
1189+
'''
1190+
1191+
def variables = [
1192+
var1: [[foo: "foo1", input2: [bar: 123]]],
1193+
var2: "2"
1194+
]
1195+
assertValidQuery(graphQLSchema, query, variables)
1196+
Document document = TestUtil.parseQuery(query)
1197+
NormalizedQueryTreeFactory dependencyGraph = new NormalizedQueryTreeFactory();
1198+
when:
1199+
def tree = dependencyGraph.createNormalizedQueryWithRawVariables(graphQLSchema, document, null, variables)
1200+
def topLevelField = tree.getTopLevelFields().get(0)
1201+
def arg1 = topLevelField.getNormalizedArgument("arg1")
1202+
def arg2 = topLevelField.getNormalizedArgument("arg2")
1203+
1204+
then:
1205+
arg1.typeName == "[[Input1]]"
1206+
arg1.value == [[
1207+
[foo: new NormalizedInputValue("String", "foo1"), input2: new NormalizedInputValue("Input2", [bar: new NormalizedInputValue("Int", 123)])],
1208+
]]
1209+
arg2.typeName == "[[ID!]!]"
1210+
arg2.value == [["1"], ["2"]]
1211+
}
1212+
11681213
}

0 commit comments

Comments
 (0)