Skip to content

Commit 908e61e

Browse files
tinnoubbakerman
authored andcommitted
Fix ClassCastException in ExecutionStrategy.toIterable
1 parent 9dfacb2 commit 908e61e

4 files changed

Lines changed: 233 additions & 14 deletions

File tree

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package graphql;
2+
3+
import java.util.HashMap;
4+
import java.util.List;
5+
import java.util.Map;
6+
7+
import graphql.execution.ExecutionPath;
8+
import graphql.introspection.Introspection;
9+
import graphql.language.SourceLocation;
10+
import graphql.schema.GraphQLEnumType;
11+
import graphql.schema.GraphQLInputObjectType;
12+
import graphql.schema.GraphQLInterfaceType;
13+
import graphql.schema.GraphQLList;
14+
import graphql.schema.GraphQLNonNull;
15+
import graphql.schema.GraphQLObjectType;
16+
import graphql.schema.GraphQLScalarType;
17+
import graphql.schema.GraphQLType;
18+
import graphql.schema.GraphQLUnionType;
19+
20+
import static graphql.Assert.assertNotNull;
21+
import static java.lang.String.format;
22+
23+
@PublicApi
24+
public class TypeMismatchError implements GraphQLError {
25+
26+
private final String message;
27+
private final List<Object> path;
28+
private final GraphQLType expectedType;
29+
30+
public TypeMismatchError(ExecutionPath path, GraphQLType expectedType) {
31+
this.path = assertNotNull(path).toList();
32+
this.expectedType = assertNotNull(expectedType);
33+
this.message = mkMessage(path, expectedType);
34+
}
35+
36+
private String mkMessage(ExecutionPath path, GraphQLType expectedType) {
37+
String expectedTypeKind = GraphQLTypeToTypeKindMapping.getTypeKindFromGraphQLType(expectedType).name();
38+
return format("Can't resolve value (%s) : type mismatch error, expected type %s", path, expectedTypeKind);
39+
}
40+
41+
static class GraphQLTypeToTypeKindMapping {
42+
43+
private static final Map<Class<? extends GraphQLType>, Introspection.TypeKind> registry = new HashMap<Class<? extends GraphQLType>, Introspection.TypeKind>() {{
44+
put(GraphQLEnumType.class, Introspection.TypeKind.ENUM);
45+
put(GraphQLList.class, Introspection.TypeKind.LIST);
46+
put(GraphQLObjectType.class, Introspection.TypeKind.OBJECT);
47+
put(GraphQLScalarType.class, Introspection.TypeKind.SCALAR);
48+
put(GraphQLInputObjectType.class, Introspection.TypeKind.INPUT_OBJECT);
49+
put(GraphQLInterfaceType.class, Introspection.TypeKind.INTERFACE);
50+
put(GraphQLNonNull.class, Introspection.TypeKind.NON_NULL);
51+
put(GraphQLUnionType.class, Introspection.TypeKind.UNION);
52+
}};
53+
54+
private GraphQLTypeToTypeKindMapping() {
55+
}
56+
57+
public static Introspection.TypeKind getTypeKindFromGraphQLType(GraphQLType type) {
58+
return registry.containsKey(type.getClass()) ? registry.get(type.getClass()) : Assert.assertShouldNeverHappen("Unknown kind of type: " + type);
59+
}
60+
}
61+
62+
@Override
63+
public String getMessage() {
64+
return message;
65+
}
66+
67+
@Override
68+
public List<SourceLocation> getLocations() {
69+
return null;
70+
}
71+
72+
@Override
73+
public ErrorType getErrorType() {
74+
return ErrorType.DataFetchingException;
75+
}
76+
77+
public List<Object> getPath() {
78+
return path;
79+
}
80+
81+
@Override
82+
public String toString() {
83+
return "ExceptionWhileDataFetching{" +
84+
"path=" + path +
85+
"expectedType=" + expectedType +
86+
'}';
87+
}
88+
89+
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
90+
@Override
91+
public boolean equals(Object o) {
92+
return GraphqlErrorHelper.equals(this, o);
93+
}
94+
95+
@Override
96+
public int hashCode() {
97+
return GraphqlErrorHelper.hashCode(this);
98+
}
99+
}

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

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,21 @@
11
package graphql.execution;
22

3+
import java.lang.reflect.Array;
4+
import java.util.ArrayList;
5+
import java.util.List;
6+
import java.util.Map;
7+
import java.util.Optional;
8+
import java.util.concurrent.CompletableFuture;
9+
import java.util.concurrent.CompletionException;
10+
import java.util.stream.IntStream;
11+
import org.slf4j.Logger;
12+
import org.slf4j.LoggerFactory;
13+
314
import graphql.ExecutionResult;
415
import graphql.ExecutionResultImpl;
516
import graphql.PublicSpi;
617
import graphql.SerializationError;
18+
import graphql.TypeMismatchError;
719
import graphql.TypeResolutionEnvironment;
820
import graphql.execution.instrumentation.Instrumentation;
921
import graphql.execution.instrumentation.InstrumentationContext;
@@ -28,17 +40,6 @@
2840
import graphql.schema.GraphQLType;
2941
import graphql.schema.GraphQLUnionType;
3042
import graphql.schema.visibility.GraphqlFieldVisibility;
31-
import org.slf4j.Logger;
32-
import org.slf4j.LoggerFactory;
33-
34-
import java.lang.reflect.Array;
35-
import java.util.ArrayList;
36-
import java.util.List;
37-
import java.util.Map;
38-
import java.util.Optional;
39-
import java.util.concurrent.CompletableFuture;
40-
import java.util.concurrent.CompletionException;
41-
import java.util.stream.IntStream;
4243

4344
import static graphql.execution.ExecutionTypeInfo.newTypeInfo;
4445
import static graphql.execution.FieldCollectorParameters.newParameters;
@@ -354,7 +355,7 @@ protected CompletableFuture<ExecutionResult> completeValue(ExecutionContext exec
354355
if (result == null) {
355356
return completedFuture(new ExecutionResultImpl(parameters.nonNullFieldValidator().validate(parameters.path(), null), null));
356357
} else if (fieldType instanceof GraphQLList) {
357-
return completeValueForList(executionContext, parameters, toIterable(result));
358+
return completeValueForList(executionContext, parameters, result);
358359
} else if (fieldType instanceof GraphQLScalarType) {
359360
return completeValueForScalar(executionContext, parameters, (GraphQLScalarType) fieldType, result);
360361
} else if (fieldType instanceof GraphQLEnumType) {
@@ -552,7 +553,41 @@ private Object handleCoercionProblem(ExecutionContext context, ExecutionStrategy
552553
*
553554
* @param executionContext contains the top level execution parameters
554555
* @param parameters contains the parameters holding the fields to be executed and source object
555-
* @param iterableValues the values to complete
556+
* @param result the result to complete, raw result
557+
*
558+
* @return an {@link ExecutionResult}
559+
*/
560+
protected CompletableFuture<ExecutionResult> completeValueForList(ExecutionContext executionContext, ExecutionStrategyParameters parameters, Object result) {
561+
Iterable<Object> resultIterable = toIterable(executionContext, parameters, result);
562+
resultIterable = parameters.nonNullFieldValidator().validate(parameters.path(), resultIterable);
563+
if (resultIterable == null) {
564+
return completedFuture(new ExecutionResultImpl(null, null));
565+
}
566+
return completeValueForList(executionContext, parameters, resultIterable);
567+
}
568+
569+
protected Iterable<Object> toIterable(ExecutionContext context, ExecutionStrategyParameters parameters, Object result) {
570+
if (result.getClass().isArray() || result instanceof Iterable) {
571+
return toIterable(result);
572+
}
573+
574+
handleTypeMismatchProblem(context, parameters, result);
575+
return null;
576+
}
577+
578+
private void handleTypeMismatchProblem(ExecutionContext context, ExecutionStrategyParameters parameters, Object result) {
579+
TypeMismatchError error = new TypeMismatchError(parameters.path(), parameters.typeInfo().getType());
580+
log.warn("{} got {}", error.getMessage(), result.getClass());
581+
context.addError(error, parameters.path());
582+
}
583+
584+
/**
585+
* Called to complete a list of value for a field based on a list type. This iterates the values and calls
586+
* {@link #completeValue(ExecutionContext, ExecutionStrategyParameters)} for each value.
587+
*
588+
* @param executionContext contains the top level execution parameters
589+
* @param parameters contains the parameters holding the fields to be executed and source object
590+
* @param iterableValues the values to complete, can't be null
556591
*
557592
* @return an {@link ExecutionResult}
558593
*/
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package graphql
2+
3+
import graphql.introspection.Introspection
4+
import graphql.schema.*
5+
import spock.lang.Specification
6+
import spock.lang.Unroll
7+
8+
class TypeMismatchErrorTest extends Specification {
9+
10+
@Unroll
11+
def "test GraphQLTypeToTypeKindMapping mapping #type.getClass().getSimpleName()"(GraphQLType type, Introspection.TypeKind typeKind) {
12+
expect:
13+
TypeMismatchError.GraphQLTypeToTypeKindMapping.getTypeKindFromGraphQLType(type) == typeKind
14+
15+
where:
16+
type || typeKind
17+
new GraphQLList(Scalars.GraphQLInt) || Introspection.TypeKind.LIST
18+
Scalars.GraphQLInt || Introspection.TypeKind.SCALAR
19+
new GraphQLObjectType("myObject", "...", [], []) || Introspection.TypeKind.OBJECT
20+
new GraphQLEnumType("myEnum", "...", []) || Introspection.TypeKind.ENUM
21+
new GraphQLInputObjectType("myInputType", "...", []) || Introspection.TypeKind.INPUT_OBJECT
22+
new GraphQLInterfaceType("myInterfaceType", "...", [], new TypeResolverProxy()) || Introspection.TypeKind.INTERFACE
23+
new GraphQLNonNull(Scalars.GraphQLInt) || Introspection.TypeKind.NON_NULL
24+
new GraphQLUnionType("myUnion", "...", [Scalars.GraphQLInt], new TypeResolverProxy()) || Introspection.TypeKind.UNION
25+
}
26+
}

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import graphql.ExceptionWhileDataFetching
66
import graphql.ExecutionResult
77
import graphql.Scalars
88
import graphql.SerializationError
9+
import graphql.TypeMismatchError
910
import graphql.execution.instrumentation.NoOpInstrumentation
1011
import graphql.language.Argument
1112
import graphql.language.Field
@@ -112,11 +113,14 @@ class ExecutionStrategyTest extends Specification {
112113
def fieldType = new GraphQLList(GraphQLString)
113114
def fldDef = newFieldDefinition().name("test").type(fieldType).build()
114115
def typeInfo = ExecutionTypeInfo.newTypeInfo().type(fieldType).fieldDefinition(fldDef).build()
116+
NonNullableFieldValidator nullableFieldValidator = new NonNullableFieldValidator(executionContext, typeInfo)
117+
115118

116119
def result = ["test", "1", "2", "3"]
117120
def parameters = newParameters()
118121
.typeInfo(typeInfo)
119122
.source(result)
123+
.nonNullFieldValidator(nullableFieldValidator)
120124
.fields(["fld": []])
121125
.build()
122126

@@ -179,10 +183,13 @@ class ExecutionStrategyTest extends Specification {
179183
ExecutionContext executionContext = buildContext()
180184
def fieldType = new GraphQLList(GraphQLString)
181185
def fldDef = newFieldDefinition().name("test").type(fieldType).build()
186+
def typeInfo = ExecutionTypeInfo.newTypeInfo().type(fieldType).fieldDefinition(fldDef).build()
187+
NonNullableFieldValidator nullableFieldValidator = new NonNullableFieldValidator(executionContext, typeInfo)
182188
def result = ["test", "1", "2", "3"]
183189
def parameters = newParameters()
184-
.typeInfo(ExecutionTypeInfo.newTypeInfo().type(fieldType).fieldDefinition(fldDef))
190+
.typeInfo(typeInfo)
185191
.source(result)
192+
.nonNullFieldValidator(nullableFieldValidator)
186193
.fields(["fld": []])
187194
.build()
188195

@@ -503,10 +510,12 @@ class ExecutionStrategyTest extends Specification {
503510
def fieldType = new GraphQLList(Scalars.GraphQLLong)
504511
def fldDef = newFieldDefinition().name("test").type(fieldType).build()
505512
def typeInfo = ExecutionTypeInfo.newTypeInfo().type(fieldType).fieldDefinition(fldDef).build()
513+
NonNullableFieldValidator nullableFieldValidator = new NonNullableFieldValidator(executionContext, typeInfo)
506514

507515
def parameters = newParameters()
508516
.typeInfo(typeInfo)
509517
.source(result)
518+
.nonNullFieldValidator(nullableFieldValidator)
510519
.fields(["fld": [new Field()]])
511520
.field([new Field()])
512521
.build()
@@ -573,4 +582,54 @@ class ExecutionStrategyTest extends Specification {
573582
executionContext.getErrors()[0].message == "bad foo"
574583
executionContext.getErrors()[0].path == null
575584
}
585+
586+
def "completes value for an iterable"() {
587+
given:
588+
ExecutionContext executionContext = buildContext()
589+
List<Long> result = [1L, 2L, 3L]
590+
def fieldType = new GraphQLList(Scalars.GraphQLLong)
591+
def fldDef = newFieldDefinition().name("test").type(fieldType).build()
592+
def typeInfo = ExecutionTypeInfo.newTypeInfo().type(fieldType).fieldDefinition(fldDef).build()
593+
NonNullableFieldValidator nullableFieldValidator = new NonNullableFieldValidator(executionContext, typeInfo)
594+
595+
def parameters = newParameters()
596+
.typeInfo(typeInfo)
597+
.source(result)
598+
.nonNullFieldValidator(nullableFieldValidator)
599+
.fields(["fld": [new Field()]])
600+
.field([new Field()])
601+
.build()
602+
603+
when:
604+
def executionResult = executionStrategy.completeValue(executionContext, parameters)
605+
606+
then:
607+
executionResult.get().data == [1L, 2L, 3L]
608+
}
609+
610+
def "when completeValue expects GraphQLList and non iterable or non array is passed then it should yield a TypeMismatch error"() {
611+
given:
612+
ExecutionContext executionContext = buildContext()
613+
Map<String, Object> result = new HashMap<>()
614+
def fieldType = new GraphQLList(Scalars.GraphQLLong)
615+
def fldDef = newFieldDefinition().name("test").type(fieldType).build()
616+
def typeInfo = ExecutionTypeInfo.newTypeInfo().type(fieldType).fieldDefinition(fldDef).build()
617+
NonNullableFieldValidator nullableFieldValidator = new NonNullableFieldValidator(executionContext, typeInfo)
618+
619+
def parameters = newParameters()
620+
.typeInfo(typeInfo)
621+
.source(result)
622+
.nonNullFieldValidator(nullableFieldValidator)
623+
.fields(["fld": [new Field()]])
624+
.field([new Field()])
625+
.build()
626+
627+
when:
628+
def executionResult = executionStrategy.completeValue(executionContext, parameters).join()
629+
630+
then:
631+
executionResult.data == null
632+
executionContext.errors.size() == 1
633+
executionContext.errors[0] instanceof TypeMismatchError
634+
}
576635
}

0 commit comments

Comments
 (0)