From 224b542c783bb2479cd5436cb8a423882c90a047 Mon Sep 17 00:00:00 2001 From: Jonathan Perlow Date: Sun, 21 Aug 2016 22:49:23 -0700 Subject: [PATCH 1/2] Better exception error messages for debugging. Summary: Better error messages when fields are missing. --- .../graphql/execution/ValuesResolver.java | 23 ++++++++++--------- .../batched/BatchedExecutionStrategy.java | 2 +- .../java/graphql/schema/GraphQLEnumType.java | 5 ++++ .../schema/GraphQLInputObjectType.java | 5 ++++ .../graphql/schema/GraphQLInterfaceType.java | 5 ++++ src/main/java/graphql/schema/GraphQLList.java | 5 ++++ .../java/graphql/schema/GraphQLNonNull.java | 5 ++++ .../graphql/schema/GraphQLObjectType.java | 4 ++++ .../graphql/schema/GraphQLScalarType.java | 4 ++++ src/main/java/graphql/schema/GraphQLType.java | 2 +- .../graphql/schema/GraphQLTypeReference.java | 5 ++++ .../java/graphql/schema/GraphQLUnionType.java | 5 ++++ 12 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index f2d70cb256..0a1ddd88ce 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -56,18 +56,18 @@ private Object getVariableValue(GraphQLSchema schema, VariableDefinition variabl return coerceValueAst(type, variableDefinition.getDefaultValue(), null); } - return coerceValue(type, inputValue); + return coerceValue(type, variableDefinition.getName(), inputValue); } private boolean isValid(GraphQLType type, Object inputValue) { return true; } - private Object coerceValue(GraphQLType graphQLType, Object value) { + private Object coerceValue(GraphQLType graphQLType, String fieldName, Object value) { if (graphQLType instanceof GraphQLNonNull) { - Object returnValue = coerceValue(((GraphQLNonNull) graphQLType).getWrappedType(), value); + Object returnValue = coerceValue(((GraphQLNonNull) graphQLType).getWrappedType(), fieldName, value); if (returnValue == null) { - throw new GraphQLException("Null value for NonNull type " + graphQLType); + throw new GraphQLException("Null value for field " + fieldName + ", NonNull type " + graphQLType.getNameForErrorMessages()); } return returnValue; } @@ -79,19 +79,20 @@ private Object coerceValue(GraphQLType graphQLType, Object value) { } else if (graphQLType instanceof GraphQLEnumType) { return coerceValueForEnum((GraphQLEnumType) graphQLType, value); } else if (graphQLType instanceof GraphQLList) { - return coerceValueForList((GraphQLList) graphQLType, value); + return coerceValueForList((GraphQLList) graphQLType, fieldName, value); } else if (graphQLType instanceof GraphQLInputObjectType) { return coerceValueForInputObjectType((GraphQLInputObjectType) graphQLType, (Map) value); } else { - throw new GraphQLException("unknown type " + graphQLType); + throw new GraphQLException("unknown type " + (graphQLType != null ? graphQLType.getNameForErrorMessages() : null)); } } private Object coerceValueForInputObjectType(GraphQLInputObjectType inputObjectType, Map input) { Map result = new LinkedHashMap(); for (GraphQLInputObjectField inputField : inputObjectType.getFields()) { - Object value = coerceValue(inputField.getType(), input.get(inputField.getName())); - result.put(inputField.getName(), value == null ? inputField.getDefaultValue() : value); + String fieldName = inputField.getName(); + Object value = coerceValue(inputField.getType(), fieldName, input.get(fieldName)); + result.put(fieldName, value == null ? inputField.getDefaultValue() : value); } return result; @@ -105,15 +106,15 @@ private Object coerceValueForEnum(GraphQLEnumType graphQLEnumType, Object value) return graphQLEnumType.getCoercing().parseValue(value); } - private List coerceValueForList(GraphQLList graphQLList, Object value) { + private List coerceValueForList(GraphQLList graphQLList, String fieldName, Object value) { if (value instanceof Iterable) { List result = new ArrayList(); for (Object val : (Iterable) value) { - result.add(coerceValue(graphQLList.getWrappedType(), val)); + result.add(coerceValue(graphQLList.getWrappedType(), fieldName, val)); } return result; } else { - return Collections.singletonList(coerceValue(graphQLList.getWrappedType(), value)); + return Collections.singletonList(coerceValue(graphQLList.getWrappedType(), fieldName, value)); } } diff --git a/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java b/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java index 16e40e655d..f5ae234d34 100644 --- a/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java +++ b/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java @@ -168,7 +168,7 @@ private GraphQLType handleNonNullType(GraphQLType fieldType, List"; + } } diff --git a/src/main/java/graphql/schema/GraphQLNonNull.java b/src/main/java/graphql/schema/GraphQLNonNull.java index 167f3ddef5..bcef7e3dc6 100644 --- a/src/main/java/graphql/schema/GraphQLNonNull.java +++ b/src/main/java/graphql/schema/GraphQLNonNull.java @@ -49,4 +49,9 @@ public String toString() { public String getName() { return null; } + + @Override + public String getNameForErrorMessages() { + return wrappedType.getNameForErrorMessages(); + } } diff --git a/src/main/java/graphql/schema/GraphQLObjectType.java b/src/main/java/graphql/schema/GraphQLObjectType.java index 04e19b4672..cfb74fabd2 100644 --- a/src/main/java/graphql/schema/GraphQLObjectType.java +++ b/src/main/java/graphql/schema/GraphQLObjectType.java @@ -59,6 +59,10 @@ public String getName() { return name; } + @Override + public String getNameForErrorMessages() { + return name; + } @Override public String toString() { diff --git a/src/main/java/graphql/schema/GraphQLScalarType.java b/src/main/java/graphql/schema/GraphQLScalarType.java index fd7bac8b86..a9b7f32833 100644 --- a/src/main/java/graphql/schema/GraphQLScalarType.java +++ b/src/main/java/graphql/schema/GraphQLScalarType.java @@ -22,6 +22,10 @@ public String getName() { return name; } + @Override + public String getNameForErrorMessages() { + return name; + } public String getDescription() { return description; diff --git a/src/main/java/graphql/schema/GraphQLType.java b/src/main/java/graphql/schema/GraphQLType.java index 51bb0d7b72..fdd63d9d3d 100644 --- a/src/main/java/graphql/schema/GraphQLType.java +++ b/src/main/java/graphql/schema/GraphQLType.java @@ -5,5 +5,5 @@ public interface GraphQLType { String getName(); - + String getNameForErrorMessages(); } diff --git a/src/main/java/graphql/schema/GraphQLTypeReference.java b/src/main/java/graphql/schema/GraphQLTypeReference.java index 103e326f26..ff7d0274f2 100644 --- a/src/main/java/graphql/schema/GraphQLTypeReference.java +++ b/src/main/java/graphql/schema/GraphQLTypeReference.java @@ -19,4 +19,9 @@ public GraphQLTypeReference(String name) { public String getName() { return name; } + + @Override + public String getNameForErrorMessages() { + return name; + } } diff --git a/src/main/java/graphql/schema/GraphQLUnionType.java b/src/main/java/graphql/schema/GraphQLUnionType.java index 99d09ca322..215a929d98 100644 --- a/src/main/java/graphql/schema/GraphQLUnionType.java +++ b/src/main/java/graphql/schema/GraphQLUnionType.java @@ -39,6 +39,11 @@ public String getName() { return name; } + @Override + public String getNameForErrorMessages() { + return name; + } + public String getDescription() { return description; } From adf2c47a8d4024e0f99bda2b76431e3535d44cb7 Mon Sep 17 00:00:00 2001 From: Jonathan Perlow Date: Mon, 12 Sep 2016 16:11:27 -0700 Subject: [PATCH 2/2] Make it possible to provide an external Execution class. --- src/main/java/graphql/GraphQL.java | 6 +++--- src/main/java/graphql/execution/Execution.java | 4 ++-- .../java/graphql/execution/ExecutionContext.java | 6 +++--- .../graphql/execution/ExecutionContextBuilder.java | 2 +- .../java/graphql/execution/ExecutionStrategy.java | 4 +--- .../java/graphql/execution/IExecutionStrategy.java | 12 ++++++++++++ .../execution/batched/GraphQLExecutionNode.java | 2 +- .../execution/batched/GraphQLExecutionNodeDatum.java | 2 +- .../batched/GraphQLExecutionResultContainer.java | 2 +- 9 files changed, 25 insertions(+), 15 deletions(-) create mode 100644 src/main/java/graphql/execution/IExecutionStrategy.java diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 18d9bbd61f..dec80138e0 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -2,7 +2,7 @@ import graphql.execution.Execution; -import graphql.execution.ExecutionStrategy; +import graphql.execution.IExecutionStrategy; import graphql.language.Document; import graphql.language.SourceLocation; import graphql.parser.Parser; @@ -25,7 +25,7 @@ public class GraphQL { private final GraphQLSchema graphQLSchema; - private final ExecutionStrategy executionStrategy; + private final IExecutionStrategy executionStrategy; private static final Logger log = LoggerFactory.getLogger(GraphQL.class); @@ -34,7 +34,7 @@ public GraphQL(GraphQLSchema graphQLSchema) { } - public GraphQL(GraphQLSchema graphQLSchema, ExecutionStrategy executionStrategy) { + public GraphQL(GraphQLSchema graphQLSchema, IExecutionStrategy executionStrategy) { this.graphQLSchema = graphQLSchema; this.executionStrategy = executionStrategy; } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index cf220c06ad..01900f8372 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -17,9 +17,9 @@ public class Execution { private FieldCollector fieldCollector = new FieldCollector(); - private ExecutionStrategy strategy; + private IExecutionStrategy strategy; - public Execution(ExecutionStrategy strategy) { + public Execution(IExecutionStrategy strategy) { this.strategy = strategy; if (this.strategy == null) { diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index c2f09dd41d..ea0868d460 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -14,7 +14,7 @@ public class ExecutionContext { private GraphQLSchema graphQLSchema; - private ExecutionStrategy executionStrategy; + private IExecutionStrategy executionStrategy; private Map fragmentsByName = new LinkedHashMap(); private OperationDefinition operationDefinition; private Map variables = new LinkedHashMap(); @@ -73,11 +73,11 @@ public List getErrors() { return errors; } - public ExecutionStrategy getExecutionStrategy() { + public IExecutionStrategy getExecutionStrategy() { return executionStrategy; } - public void setExecutionStrategy(ExecutionStrategy executionStrategy) { + public void setExecutionStrategy(IExecutionStrategy executionStrategy) { this.executionStrategy = executionStrategy; } } diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index d16ac1ecbf..20c8041efc 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -18,7 +18,7 @@ public ExecutionContextBuilder(ValuesResolver valuesResolver) { this.valuesResolver = valuesResolver; } - public ExecutionContext build(GraphQLSchema graphQLSchema, ExecutionStrategy executionStrategy, Object root, Document document, String operationName, Map args) { + public ExecutionContext build(GraphQLSchema graphQLSchema, IExecutionStrategy executionStrategy, Object root, Document document, String operationName, Map args) { Map fragmentsByName = new LinkedHashMap(); Map operationsByName = new LinkedHashMap(); diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 882f30f7bd..5ee36fbf75 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -13,14 +13,12 @@ import static graphql.introspection.Introspection.*; -public abstract class ExecutionStrategy { +public abstract class ExecutionStrategy implements IExecutionStrategy { private static final Logger log = LoggerFactory.getLogger(ExecutionStrategy.class); protected ValuesResolver valuesResolver = new ValuesResolver(); protected FieldCollector fieldCollector = new FieldCollector(); - public abstract ExecutionResult execute(ExecutionContext executionContext, GraphQLObjectType parentType, Object source, Map> fields); - protected ExecutionResult resolveField(ExecutionContext executionContext, GraphQLObjectType parentType, Object source, List fields) { GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, fields.get(0)); diff --git a/src/main/java/graphql/execution/IExecutionStrategy.java b/src/main/java/graphql/execution/IExecutionStrategy.java new file mode 100644 index 0000000000..c3bed3a378 --- /dev/null +++ b/src/main/java/graphql/execution/IExecutionStrategy.java @@ -0,0 +1,12 @@ +package graphql.execution; + +import graphql.ExecutionResult; +import graphql.language.Field; +import graphql.schema.GraphQLObjectType; + +import java.util.List; +import java.util.Map; + +public interface IExecutionStrategy { + ExecutionResult execute(ExecutionContext executionContext, GraphQLObjectType parentType, Object source, Map> fields); +} diff --git a/src/main/java/graphql/execution/batched/GraphQLExecutionNode.java b/src/main/java/graphql/execution/batched/GraphQLExecutionNode.java index da88f04fcc..87cddb5817 100644 --- a/src/main/java/graphql/execution/batched/GraphQLExecutionNode.java +++ b/src/main/java/graphql/execution/batched/GraphQLExecutionNode.java @@ -6,7 +6,7 @@ import java.util.List; import java.util.Map; -class GraphQLExecutionNode { +public class GraphQLExecutionNode { private final GraphQLObjectType parentType; private final Map> fields; diff --git a/src/main/java/graphql/execution/batched/GraphQLExecutionNodeDatum.java b/src/main/java/graphql/execution/batched/GraphQLExecutionNodeDatum.java index 61d558f154..536cb1be33 100644 --- a/src/main/java/graphql/execution/batched/GraphQLExecutionNodeDatum.java +++ b/src/main/java/graphql/execution/batched/GraphQLExecutionNodeDatum.java @@ -2,7 +2,7 @@ import java.util.Map; -class GraphQLExecutionNodeDatum extends GraphQLExecutionResultContainer { +public class GraphQLExecutionNodeDatum extends GraphQLExecutionResultContainer { private final Map parentResult; private final Object source; diff --git a/src/main/java/graphql/execution/batched/GraphQLExecutionResultContainer.java b/src/main/java/graphql/execution/batched/GraphQLExecutionResultContainer.java index 2410b5312c..931317c7a1 100644 --- a/src/main/java/graphql/execution/batched/GraphQLExecutionResultContainer.java +++ b/src/main/java/graphql/execution/batched/GraphQLExecutionResultContainer.java @@ -30,6 +30,6 @@ public GraphQLExecutionResultList createAndPutEmptyChildList(String fieldName) { * @param fieldName fieldName * @param value value */ - abstract void putResult(String fieldName, Object value); + public abstract void putResult(String fieldName, Object value); }