diff --git a/src/main/java/graphql/Assert.java b/src/main/java/graphql/Assert.java index 259b22cc5d..d5c85990ed 100644 --- a/src/main/java/graphql/Assert.java +++ b/src/main/java/graphql/Assert.java @@ -10,6 +10,11 @@ public static T assertNotNull(T object, String errorMessage) { throw new AssertException(errorMessage); } + public static T assertNotNull(T object) { + if (object != null) return object; + throw new AssertException("Object required to be not null"); + } + public static T assertNeverCalled() { throw new AssertException("Should never been called"); } @@ -39,6 +44,7 @@ public static void assertTrue(boolean condition, String errorMessage) { * currently non null, non empty, * * @param name - the name to be validated. + * * @return the name if valid, or AssertException if invalid. */ public static String assertValidName(String name) { diff --git a/src/main/java/graphql/ExceptionWhileDataFetching.java b/src/main/java/graphql/ExceptionWhileDataFetching.java index 699862c570..f125065a39 100644 --- a/src/main/java/graphql/ExceptionWhileDataFetching.java +++ b/src/main/java/graphql/ExceptionWhileDataFetching.java @@ -1,17 +1,23 @@ package graphql; +import graphql.execution.ExecutionPath; import graphql.language.SourceLocation; import java.util.List; +import static graphql.Assert.assertNotNull; +import static java.lang.String.format; + @PublicApi public class ExceptionWhileDataFetching implements GraphQLError { + private final ExecutionPath path; private final Throwable exception; - public ExceptionWhileDataFetching(Throwable exception) { - this.exception = exception; + public ExceptionWhileDataFetching(ExecutionPath path, Throwable exception) { + this.path = assertNotNull(path); + this.exception = assertNotNull(exception); } public Throwable getException() { @@ -21,7 +27,7 @@ public Throwable getException() { @Override public String getMessage() { - return "Exception while fetching data: " + exception.getMessage(); + return format("Exception while fetching data (%s) : %s", path, exception.getMessage()); } @Override @@ -29,6 +35,16 @@ public List getLocations() { return null; } + /** + * The graphql spec says that that path field of any error should be a list + * of path entries - http://facebook.github.io/graphql/#sec-Errors + * + * @return the path in list format + */ + public List getPath() { + return path.toList(); + } + @Override public ErrorType getErrorType() { return ErrorType.DataFetchingException; @@ -37,10 +53,12 @@ public ErrorType getErrorType() { @Override public String toString() { return "ExceptionWhileDataFetching{" + + "path=" + path + "exception=" + exception + '}'; } + @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") @Override public boolean equals(Object o) { return Helper.equals(this, o); diff --git a/src/main/java/graphql/Scalars.java b/src/main/java/graphql/Scalars.java index 3a84d8b9ec..493095d421 100644 --- a/src/main/java/graphql/Scalars.java +++ b/src/main/java/graphql/Scalars.java @@ -28,13 +28,7 @@ public class Scalars { private static boolean isNumberIsh(Object input) { - if (input instanceof Number) { - return true; - } - if (input instanceof String) { - return true; - } - return false; + return input instanceof Number || input instanceof String; } public static GraphQLScalarType GraphQLInt = new GraphQLScalarType("Int", "Built-in Int", new Coercing() { @@ -64,7 +58,7 @@ private Integer convertImpl(Object input) { public Integer serialize(Object input) { Integer result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid value " + input + " for Int"); + throw new CoercingSerializeException("Invalid value '" + input + "' for Int"); } return result; } @@ -73,7 +67,7 @@ public Integer serialize(Object input) { public Integer parseValue(Object input) { Integer result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid value " + input + " for Int"); + throw new CoercingParseValueException("Invalid value '" + input + "' for Int"); } return result; } @@ -115,7 +109,7 @@ private Long convertImpl(Object input) { public Long serialize(Object input) { Long result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for Long"); + throw new CoercingSerializeException("Invalid input '" + input + "' for Long"); } return result; } @@ -124,7 +118,7 @@ public Long serialize(Object input) { public Long parseValue(Object input) { Long result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for Long"); + throw new CoercingParseValueException("Invalid input '" + input + "' for Long"); } return result; } @@ -175,7 +169,7 @@ private Short convertImpl(Object input) { public Short serialize(Object input) { Short result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for Short"); + throw new CoercingSerializeException("Invalid input '" + input + "' for Short"); } return result; } @@ -184,7 +178,7 @@ public Short serialize(Object input) { public Short parseValue(Object input) { Short result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for Short"); + throw new CoercingParseValueException("Invalid input '" + input + "' for Short"); } return result; } @@ -226,7 +220,7 @@ private Byte convertImpl(Object input) { public Byte serialize(Object input) { Byte result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for Byte"); + throw new CoercingSerializeException("Invalid input '" + input + "' for Byte"); } return result; } @@ -235,7 +229,7 @@ public Byte serialize(Object input) { public Byte parseValue(Object input) { Byte result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for Byte"); + throw new CoercingParseValueException("Invalid input '" + input + "' for Byte"); } return result; } @@ -276,7 +270,7 @@ private Double convertImpl(Object input) { public Double serialize(Object input) { Double result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for Byte"); + throw new CoercingSerializeException("Invalid input '" + input + "' for Float"); } return result; @@ -286,7 +280,7 @@ public Double serialize(Object input) { public Double parseValue(Object input) { Double result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for Byte"); + throw new CoercingParseValueException("Invalid input '" + input + "' for Float"); } return result; } @@ -327,7 +321,7 @@ private BigInteger convertImpl(Object input) { public BigInteger serialize(Object input) { BigInteger result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for BigDecimal"); + throw new CoercingSerializeException("Invalid input '" + input + "' for BigInteger"); } return result; } @@ -336,7 +330,7 @@ public BigInteger serialize(Object input) { public BigInteger parseValue(Object input) { BigInteger result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for BigDecimal"); + throw new CoercingParseValueException("Invalid input '" + input + "' for BigInteger"); } return result; } @@ -380,7 +374,7 @@ private BigDecimal convertImpl(Object input) { public BigDecimal serialize(Object input) { BigDecimal result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for BigDecimal"); + throw new CoercingSerializeException("Invalid input '" + input + "' for BigDecimal"); } return result; } @@ -389,7 +383,7 @@ public BigDecimal serialize(Object input) { public BigDecimal parseValue(Object input) { BigDecimal result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for BigDecimal"); + throw new CoercingParseValueException("Invalid input '" + input + "' for BigDecimal"); } return result; } @@ -457,7 +451,7 @@ private Boolean convertImpl(Object input) { public Boolean serialize(Object input) { Boolean result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for Boolean"); + throw new CoercingSerializeException("Invalid input '" + input + "' for Boolean"); } return result; } @@ -466,7 +460,7 @@ public Boolean serialize(Object input) { public Boolean parseValue(Object input) { Boolean result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for Boolean"); + throw new CoercingParseValueException("Invalid input '" + input + "' for Boolean"); } return result; } @@ -498,7 +492,7 @@ private String convertImpl(Object input) { public String serialize(Object input) { String result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for ID"); + throw new CoercingSerializeException("Invalid input '" + input + "' for ID"); } return result; } @@ -507,7 +501,7 @@ public String serialize(Object input) { public String parseValue(Object input) { String result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for ID"); + throw new CoercingParseValueException("Invalid input '" + input + "' for ID"); } return result; } @@ -542,7 +536,7 @@ private Character convertImpl(Object input) { public Character serialize(Object input) { Character result = convertImpl(input); if (result == null) { - throw new CoercingSerializeException("Invalid input " + input + " for Char"); + throw new CoercingSerializeException("Invalid input '" + input + "' for Char"); } return result; } @@ -551,7 +545,7 @@ public Character serialize(Object input) { public Character parseValue(Object input) { Character result = convertImpl(input); if (result == null) { - throw new CoercingParseValueException("Invalid input " + input + " for Char"); + throw new CoercingParseValueException("Invalid input '" + input + "' for Char"); } return result; } diff --git a/src/main/java/graphql/SerializationError.java b/src/main/java/graphql/SerializationError.java index d0e63161d1..564d6d25da 100644 --- a/src/main/java/graphql/SerializationError.java +++ b/src/main/java/graphql/SerializationError.java @@ -1,18 +1,24 @@ package graphql; +import graphql.execution.ExecutionPath; import graphql.language.SourceLocation; import graphql.schema.CoercingSerializeException; import java.util.List; +import static graphql.Assert.assertNotNull; +import static java.lang.String.format; + @PublicApi public class SerializationError implements GraphQLError { private final CoercingSerializeException exception; + private final ExecutionPath path; - public SerializationError(CoercingSerializeException exception) { - this.exception = exception; + public SerializationError(ExecutionPath path, CoercingSerializeException exception) { + this.path = assertNotNull(path); + this.exception = assertNotNull(exception); } public CoercingSerializeException getException() { @@ -22,7 +28,7 @@ public CoercingSerializeException getException() { @Override public String getMessage() { - return "Can't serialize value: " + exception.getMessage(); + return format("Can't serialize value (%s) : %s", path, exception.getMessage()); } @Override @@ -35,13 +41,25 @@ public ErrorType getErrorType() { return ErrorType.DataFetchingException; } + /** + * The graphql spec says that that path field of any error should be a list + * of path entries - http://facebook.github.io/graphql/#sec-Errors + * + * @return the path in list format + */ + public List getPath() { + return path.toList(); + } + @Override public String toString() { return "ExceptionWhileDataFetching{" + + "path=" + path + "exception=" + exception + '}'; } + @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") @Override public boolean equals(Object o) { return Helper.equals(this, o); diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 105495b313..2a9c6c08ec 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -98,6 +98,7 @@ private ExecutionResult executeOperation( .source(root) .fields(fields) .nonNullFieldValidator(nonNullableFieldValidator) + .path(ExecutionPath.rootPath()) .build(); ExecutionResult result; diff --git a/src/main/java/graphql/execution/ExecutionPath.java b/src/main/java/graphql/execution/ExecutionPath.java new file mode 100644 index 0000000000..de5b582430 --- /dev/null +++ b/src/main/java/graphql/execution/ExecutionPath.java @@ -0,0 +1,109 @@ +package graphql.execution; + +import graphql.Internal; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static graphql.Assert.assertNotNull; +import static graphql.Assert.assertTrue; + + +@Internal +public class ExecutionPath { + private static ExecutionPath ROOT_PATH = new ExecutionPath(); + + public static ExecutionPath rootPath() { + return ROOT_PATH; + } + + private final ExecutionPath parent; + private final PathSegment segment; + + private ExecutionPath() { + parent = null; + segment = null; + } + + private ExecutionPath(ExecutionPath parent, PathSegment segment) { + this.parent = assertNotNull(parent, "Must provide a parent path"); + this.segment = assertNotNull(segment, "Must provide a sub path"); + } + + public ExecutionPath segment(String segment) { + return new ExecutionPath(this, new StringPathSegment(segment)); + } + + public ExecutionPath segment(int segment) { + return new ExecutionPath(this, new IntPathSegment(segment)); + } + + public List toList() { + if (parent == null) { + return Collections.emptyList(); + } + List list = new ArrayList<>(); + ExecutionPath p = this; + while (p.segment != null) { + list.add(p.segment.getValue()); + p = p.parent; + } + Collections.reverse(list); + return list; + } + + @Override + public String toString() { + if (parent == null) { + return ""; + } + + if (parent == ROOT_PATH) { + return segment.toString(); + } + + return parent.toString() + segment.toString(); + } + + public interface PathSegment { + T getValue(); + } + + private static class StringPathSegment implements PathSegment { + private final String value; + + public StringPathSegment(String value) { + assertTrue(value != null && !value.isEmpty(), "empty path component"); + this.value = value; + } + + @Override + public String getValue() { + return value; + } + + @Override + public String toString() { + return '/' + value; + } + } + + private static class IntPathSegment implements PathSegment { + private final int value; + + public IntPathSegment(int value) { + this.value = value; + } + + @Override + public Integer getValue() { + return value; + } + + @Override + public String toString() { + return "[" + value + ']'; + } + } +} diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index c7e3303e9c..1274198ca4 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -60,6 +60,7 @@ public abstract class ExecutionStrategy { * @param executionContext the execution context in play * @param fieldDef the field definition * @param argumentValues the map of arguments + * @param path the logical path to the field in question * @param e the exception that occurred */ @SuppressWarnings("unused") @@ -67,8 +68,12 @@ protected void handleDataFetchingException( ExecutionContext executionContext, GraphQLFieldDefinition fieldDef, Map argumentValues, + ExecutionPath path, Exception e) { - executionContext.addError(new ExceptionWhileDataFetching(e)); + ExceptionWhileDataFetching error = new ExceptionWhileDataFetching(path, e); + executionContext.addError(error); + log.warn(error.getMessage(), e); + } @@ -106,9 +111,8 @@ protected ExecutionResult resolveField(ExecutionContext executionContext, Execut fetchCtx.onEnd(resolvedValue); } catch (Exception e) { - log.warn("Exception while fetching data", e); - handleDataFetchingException(executionContext, fieldDef, argumentValues, e); fetchCtx.onEnd(e); + handleDataFetchingException(executionContext, fieldDef, argumentValues, parameters.path(), e); } TypeInfo fieldTypeInfo = newTypeInfo() @@ -125,6 +129,7 @@ protected ExecutionResult resolveField(ExecutionContext executionContext, Execut .arguments(argumentValues) .source(resolvedValue) .nonNullFieldValidator(nonNullableFieldValidator) + .path(parameters.path()) .build(); ExecutionResult result = completeValue(executionContext, newParameters, fields); @@ -139,7 +144,7 @@ protected ExecutionResult completeValue(ExecutionContext executionContext, Execu GraphQLType fieldType = parameters.typeInfo().type(); if (result == null) { - return parameters.nonNullFieldValidator().validate(null); + return parameters.nonNullFieldValidator().validate(parameters.path(), null); } else if (fieldType instanceof GraphQLList) { return completeValueForList(executionContext, parameters, fields, toIterable(result)); } else if (fieldType instanceof GraphQLScalarType) { @@ -188,6 +193,7 @@ protected ExecutionResult completeValue(ExecutionContext executionContext, Execu .typeInfo(newTypeInfo) .fields(subFields) .nonNullFieldValidator(nonNullableFieldValidator) + .path(parameters.path()) .source(result).build(); // Calling this from the executionContext to ensure we shift back from mutation strategy to the query strategy. @@ -226,10 +232,9 @@ protected ExecutionResult completeValueForEnum(ExecutionContext executionContext try { serialized = enumType.getCoercing().serialize(result); } catch (CoercingSerializeException e) { - executionContext.addError(new SerializationError(e)); - serialized = null; + serialized = handleCoercionProblem(executionContext, parameters, e); } - serialized = parameters.nonNullFieldValidator().validate(serialized); + serialized = parameters.nonNullFieldValidator().validate(parameters.path(), serialized); return new ExecutionResultImpl(serialized, null); } @@ -238,8 +243,7 @@ protected ExecutionResult completeValueForScalar(ExecutionContext executionConte try { serialized = scalarType.getCoercing().serialize(result); } catch (CoercingSerializeException e) { - serialized = null; - executionContext.addError(new SerializationError(e)); + serialized = handleCoercionProblem(executionContext, parameters, e); } // TODO: fix that: this should not be handled here @@ -247,27 +251,39 @@ protected ExecutionResult completeValueForScalar(ExecutionContext executionConte if (serialized instanceof Double && ((Double) serialized).isNaN()) { serialized = null; } - serialized = parameters.nonNullFieldValidator().validate(serialized); + serialized = parameters.nonNullFieldValidator().validate(parameters.path(), serialized); return new ExecutionResultImpl(serialized, null); } + private Object handleCoercionProblem(ExecutionContext context, ExecutionStrategyParameters parameters, CoercingSerializeException e) { + SerializationError error = new SerializationError(parameters.path(), e); + log.warn(error.getMessage(), e); + context.addError(error); + return null; + } + protected ExecutionResult completeValueForList(ExecutionContext executionContext, ExecutionStrategyParameters parameters, List fields, Iterable result) { List completedResults = new ArrayList<>(); TypeInfo typeInfo = parameters.typeInfo(); GraphQLList fieldType = typeInfo.castType(GraphQLList.class); + int idx = 0; for (Object item : result) { TypeInfo wrappedTypeInfo = typeInfo.asType(fieldType.getWrappedType()); NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, wrappedTypeInfo); + ExecutionPath indexedPath = parameters.path().segment(idx); + ExecutionStrategyParameters newParameters = ExecutionStrategyParameters.newParameters() .typeInfo(wrappedTypeInfo) .fields(parameters.fields()) .nonNullFieldValidator(nonNullableFieldValidator) + .path(indexedPath) .source(item).build(); ExecutionResult completedValue = completeValue(executionContext, newParameters, fields); completedResults.add(completedValue != null ? completedValue.getData() : null); + idx++; } return new ExecutionResultImpl(completedResults, null); } diff --git a/src/main/java/graphql/execution/ExecutionStrategyParameters.java b/src/main/java/graphql/execution/ExecutionStrategyParameters.java index e8cb7d52d8..2dfc5ddb6a 100644 --- a/src/main/java/graphql/execution/ExecutionStrategyParameters.java +++ b/src/main/java/graphql/execution/ExecutionStrategyParameters.java @@ -20,13 +20,15 @@ public class ExecutionStrategyParameters { private final Map arguments; private final Map> fields; private final NonNullableFieldValidator nonNullableFieldValidator; + private final ExecutionPath path; - private ExecutionStrategyParameters(TypeInfo typeInfo, Object source, Map> fields, Map arguments, NonNullableFieldValidator nonNullableFieldValidator) { + private ExecutionStrategyParameters(TypeInfo typeInfo, Object source, Map> fields, Map arguments, NonNullableFieldValidator nonNullableFieldValidator, ExecutionPath path) { this.typeInfo = assertNotNull(typeInfo, "typeInfo is null"); this.fields = assertNotNull(fields, "fields is null"); this.source = source; this.arguments = arguments; this.nonNullableFieldValidator = nonNullableFieldValidator; + this.path = path; } public TypeInfo typeInfo() { @@ -49,12 +51,17 @@ public NonNullableFieldValidator nonNullFieldValidator() { return nonNullableFieldValidator; } + public ExecutionPath path() { + return path; + } + public ExecutionStrategyParameters transform(Consumer builderConsumer) { Builder builder = newParameters(this); builderConsumer.accept(builder); return builder.build(); } + public static Builder newParameters() { return new Builder(); } @@ -65,8 +72,8 @@ public static Builder newParameters(ExecutionStrategyParameters oldParameters) { @Override public String toString() { - return String.format("ExecutionStrategyParameters { typeInfo=%s, source=%s, fields=%s }", - typeInfo, source, fields); + return String.format("ExecutionStrategyParameters { path=%s, typeInfo=%s, source=%s, fields=%s }", + path, typeInfo, source, fields); } public static class Builder { @@ -75,6 +82,7 @@ public static class Builder { Map> fields; Map arguments; NonNullableFieldValidator nonNullableFieldValidator; + ExecutionPath path = ExecutionPath.rootPath(); private Builder() { } @@ -111,14 +119,19 @@ public Builder arguments(Map arguments) { this.arguments = arguments; return this; } - + public Builder nonNullFieldValidator(NonNullableFieldValidator nonNullableFieldValidator) { - this.nonNullableFieldValidator = Assert.assertNotNull(nonNullableFieldValidator,"requires a NonNullValidator"); + this.nonNullableFieldValidator = Assert.assertNotNull(nonNullableFieldValidator, "requires a NonNullValidator"); + return this; + } + + public Builder path(ExecutionPath path) { + this.path = path; return this; } public ExecutionStrategyParameters build() { - return new ExecutionStrategyParameters(typeInfo, source, fields, arguments, nonNullableFieldValidator); + return new ExecutionStrategyParameters(typeInfo, source, fields, arguments, nonNullableFieldValidator, path); } } } diff --git a/src/main/java/graphql/execution/ExecutorServiceExecutionStrategy.java b/src/main/java/graphql/execution/ExecutorServiceExecutionStrategy.java index 28a49adc75..f3c90cf043 100644 --- a/src/main/java/graphql/execution/ExecutorServiceExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutorServiceExecutionStrategy.java @@ -46,7 +46,11 @@ public ExecutionResult execute(final ExecutionContext executionContext, final Ex Map> futures = new LinkedHashMap<>(); for (String fieldName : fields.keySet()) { final List fieldList = fields.get(fieldName); - Callable resolveField = () -> resolveField(executionContext, parameters, fieldList); + + ExecutionPath fieldPath = parameters.path().segment(fieldName); + ExecutionStrategyParameters newParameters = parameters.transform(bldr -> bldr.path(fieldPath)); + + Callable resolveField = () -> resolveField(executionContext, newParameters, fieldList); futures.put(fieldName, executorService.submit(resolveField)); } try { diff --git a/src/main/java/graphql/execution/NonNullableFieldValidator.java b/src/main/java/graphql/execution/NonNullableFieldValidator.java index 1d347b4621..085ae705e7 100644 --- a/src/main/java/graphql/execution/NonNullableFieldValidator.java +++ b/src/main/java/graphql/execution/NonNullableFieldValidator.java @@ -8,6 +8,7 @@ * See: http://facebook.github.io/graphql/#sec-Errors-and-Non-Nullability */ public class NonNullableFieldValidator { + private final ExecutionContext executionContext; private final TypeInfo typeInfo; @@ -19,17 +20,18 @@ public NonNullableFieldValidator(ExecutionContext executionContext, TypeInfo typ /** * Called to check that a value is non null if the type requires it to be non null * + * @param path the path to this place * @param result the result to check * * @return the result back * * @throws NonNullableFieldWasNullException if the value is null but the type requires it to be non null */ - T validate(T result) throws NonNullableFieldWasNullException { + T validate(ExecutionPath path, T result) throws NonNullableFieldWasNullException { if (result == null) { if (typeInfo.typeIsNonNull()) { // see http://facebook.github.io/graphql/#sec-Errors-and-Non-Nullability - NonNullableFieldWasNullException nonNullException = new NonNullableFieldWasNullException(typeInfo); + NonNullableFieldWasNullException nonNullException = new NonNullableFieldWasNullException(typeInfo, path); executionContext.addError(nonNullException); throw nonNullException; } diff --git a/src/main/java/graphql/execution/NonNullableFieldWasNullException.java b/src/main/java/graphql/execution/NonNullableFieldWasNullException.java index cfb9fac985..83f0a7ec07 100644 --- a/src/main/java/graphql/execution/NonNullableFieldWasNullException.java +++ b/src/main/java/graphql/execution/NonNullableFieldWasNullException.java @@ -6,6 +6,8 @@ import java.util.List; +import static graphql.Assert.assertNotNull; + /** * See (http://facebook.github.io/graphql/#sec-Errors-and-Non-Nullability), but if a non nullable field * actually resolves to a null value and the parent type is nullable then the parent must in fact become null @@ -14,18 +16,20 @@ public class NonNullableFieldWasNullException extends RuntimeException implements GraphQLError { private final TypeInfo typeInfo; + private final ExecutionPath path; - public NonNullableFieldWasNullException(TypeInfo typeInfo) { - super(buildMsg(typeInfo)); + public NonNullableFieldWasNullException(TypeInfo typeInfo, ExecutionPath path) { + super(buildMsg(assertNotNull(typeInfo), assertNotNull(path))); this.typeInfo = typeInfo; + this.path = path; } - private static String buildMsg(TypeInfo typeInfo) { + private static String buildMsg(TypeInfo typeInfo, ExecutionPath path) { if (typeInfo.hasParentType()) { - return String.format("Cannot return null for non-nullable type: '%s' within parent '%s'", typeInfo.type().getName(), typeInfo.parentTypeInfo().type().getName()); + return String.format("Cannot return null for non-nullable type: '%s' within parent '%s' (%s)", typeInfo.type().getName(), typeInfo.parentTypeInfo().type().getName(), path); } - return String.format("Cannot return null for non-nullable type: '%s' ", typeInfo.type().getName()); + return String.format("Cannot return null for non-nullable type: '%s' (%s) ", typeInfo.type().getName(), path); } public TypeInfo getTypeInfo() { @@ -41,4 +45,23 @@ public List getLocations() { public ErrorType getErrorType() { return null; } + + /** + * The graphql spec says that that path field of any error should be a list + * of path entries - http://facebook.github.io/graphql/#sec-Errors + * + * @return the path in list format + */ + public List getPath() { + return path.toList(); + } + + + @Override + public String toString() { + return "NonNullableFieldWasNullException{" + + "path=" + path + + "typeInfo=" + typeInfo + + '}'; + } } diff --git a/src/main/java/graphql/execution/SimpleExecutionStrategy.java b/src/main/java/graphql/execution/SimpleExecutionStrategy.java index 706419f38c..86fb837984 100644 --- a/src/main/java/graphql/execution/SimpleExecutionStrategy.java +++ b/src/main/java/graphql/execution/SimpleExecutionStrategy.java @@ -15,8 +15,12 @@ public ExecutionResult execute(ExecutionContext executionContext, ExecutionStrat Map results = new LinkedHashMap<>(); for (String fieldName : fields.keySet()) { List fieldList = fields.get(fieldName); + + ExecutionPath fieldPath = parameters.path().segment(fieldName); + ExecutionStrategyParameters newParameters = parameters.transform(bldr -> bldr.path(fieldPath)); + try { - ExecutionResult resolvedResult = resolveField(executionContext, parameters, fieldList); + ExecutionResult resolvedResult = resolveField(executionContext, newParameters, fieldList); results.put(fieldName, resolvedResult != null ? resolvedResult.getData() : null); } catch (NonNullableFieldWasNullException e) { diff --git a/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java b/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java index e356ee372c..14d5c1236e 100644 --- a/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java +++ b/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java @@ -7,6 +7,7 @@ import graphql.execution.ExecutionStrategy; import graphql.execution.ExecutionStrategyParameters; import graphql.execution.FieldCollectorParameters; +import graphql.execution.ExecutionPath; import graphql.execution.TypeResolutionParameters; import graphql.language.Field; import graphql.schema.DataFetcher; @@ -59,10 +60,10 @@ public ExecutionResult execute(ExecutionContext executionContext, ExecutionStrat GraphQLExecutionNodeDatum data = new GraphQLExecutionNodeDatum(new LinkedHashMap<>(), parameters.source()); GraphQLObjectType type = parameters.typeInfo().castType(GraphQLObjectType.class); GraphQLExecutionNode root = new GraphQLExecutionNode(type, parameters.fields(), singletonList(data)); - return execute(executionContext, root); + return execute(executionContext, parameters, root); } - private ExecutionResult execute(ExecutionContext executionContext, GraphQLExecutionNode root) { + private ExecutionResult execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLExecutionNode root) { Queue nodes = new ArrayDeque<>(); nodes.add(root); @@ -72,8 +73,12 @@ private ExecutionResult execute(ExecutionContext executionContext, GraphQLExecut GraphQLExecutionNode node = nodes.poll(); for (String fieldName : node.getFields().keySet()) { + + ExecutionPath fieldPath = parameters.path().segment(fieldName); + ExecutionStrategyParameters newParameters = parameters.transform(bldr -> bldr.path(fieldPath)); + List fieldList = node.getFields().get(fieldName); - List childNodes = resolveField(executionContext, node.getParentType(), + List childNodes = resolveField(executionContext, newParameters, node.getParentType(), node.getData(), fieldName, fieldList); nodes.addAll(childNodes); } @@ -90,14 +95,14 @@ private GraphQLExecutionNodeDatum getOnlyElement(List // Use the data.parentResult objects to put values into. These are either primitives or empty maps // If they were empty maps, we need that list of nodes to process - private List resolveField(ExecutionContext executionContext, GraphQLObjectType parentType, + private List resolveField(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLObjectType parentType, List nodeData, String fieldName, List fields) { GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, fields.get(0)); if (fieldDef == null) { return Collections.emptyList(); } - List values = fetchData(executionContext, parentType, nodeData, fields, fieldDef); + List values = fetchData(executionContext, parameters, parentType, nodeData, fields, fieldDef); Map argumentValues = valuesResolver.getArgumentValues( fieldDef.getArguments(), fields.get(0).getArguments(), executionContext.getVariables()); @@ -280,7 +285,7 @@ private boolean isObject(GraphQLType type) { } @SuppressWarnings("unchecked") - private List fetchData(ExecutionContext executionContext, GraphQLObjectType parentType, + private List fetchData(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLObjectType parentType, List nodeData, List fields, GraphQLFieldDefinition fieldDef) { Map argumentValues = valuesResolver.getArgumentValues( @@ -312,7 +317,7 @@ private List fetchData(ExecutionContext executionCont } catch (Exception e) { values = new ArrayList<>(nodeData.size()); log.warn("Exception while fetching data", e); - handleDataFetchingException(executionContext, fieldDef, argumentValues, e); + handleDataFetchingException(executionContext, fieldDef, argumentValues, parameters.path(), e); } List retVal = new ArrayList<>(); diff --git a/src/test/groovy/graphql/ErrorsTest.groovy b/src/test/groovy/graphql/ErrorsTest.groovy index cb6b230a15..cac5c4d950 100644 --- a/src/test/groovy/graphql/ErrorsTest.groovy +++ b/src/test/groovy/graphql/ErrorsTest.groovy @@ -1,5 +1,6 @@ package graphql +import graphql.execution.ExecutionPath import graphql.language.SourceLocation import graphql.validation.ValidationError import graphql.validation.ValidationErrorType @@ -50,9 +51,9 @@ class ErrorsTest extends Specification { def "ExceptionWhileDataFetching equals and hashcode works"() { expect: - def same1 = new ExceptionWhileDataFetching(new RuntimeException("bad ju ju")) - def same2 = new ExceptionWhileDataFetching(new RuntimeException("bad ju ju")) - def different1 = new ExceptionWhileDataFetching(new RuntimeException("unexpected ju ju")) + def same1 = new ExceptionWhileDataFetching(ExecutionPath.rootPath(),new RuntimeException("bad ju ju")) + def same2 = new ExceptionWhileDataFetching(ExecutionPath.rootPath(),new RuntimeException("bad ju ju")) + def different1 = new ExceptionWhileDataFetching(ExecutionPath.rootPath(),new RuntimeException("unexpected ju ju")) commonAssert(same1, same2, different1) } diff --git a/src/test/groovy/graphql/execution/ExecutionPathTest.groovy b/src/test/groovy/graphql/execution/ExecutionPathTest.groovy new file mode 100644 index 0000000000..c60864d6d7 --- /dev/null +++ b/src/test/groovy/graphql/execution/ExecutionPathTest.groovy @@ -0,0 +1,137 @@ +package graphql.execution + +import graphql.ExceptionWhileDataFetching +import graphql.ExecutionInput +import graphql.GraphQL +import graphql.GraphQLError +import graphql.SerializationError +import graphql.TestUtil +import graphql.schema.DataFetcher +import spock.lang.Specification +import spock.lang.Unroll + +class ExecutionPathTest extends Specification { + + @Unroll + "unit test toList works as expected : #actual"() { + + expect: + actual.toList() == expected + + where: + actual || expected + ExecutionPath.rootPath() || [] + ExecutionPath.rootPath().segment("A") || ["A"] + ExecutionPath.rootPath().segment("A").segment(1).segment("B") || ["A", 1, "B"] + ExecutionPath.rootPath().segment("A").segment("B").segment(1) || ["A", "B", 1] + } + + @Unroll + "unit test toString works as expected : #actual"() { + expect: + actual.toString() == expected + + where: + actual || expected + ExecutionPath.rootPath() || "" + ExecutionPath.rootPath().segment("A") || "/A" + ExecutionPath.rootPath().segment("A").segment(1).segment("B") || "/A[1]/B" + ExecutionPath.rootPath().segment("A").segment("B").segment(1) || "/A/B[1]" + } + + + def "full integration test of path support"() { + when: + + def spec = """ + type Query { + f1 : String + f2 : [Test] + f3 : Float + f4 : NonNullType + } + + type Test { + sub1 : String + sub2 : String + } + + type NonNullType { + nonNullField : String! + } + + """ + + + def f1Fetcher = { env -> throw new RuntimeException("error") } as DataFetcher + def f2Fetcher = { env -> [false, true, false] } as DataFetcher + def f3Fetcher = { env -> "This is not a float" } as DataFetcher + def sub1Fetcher = { env -> "staticValue" } as DataFetcher + def sub2Fetcher = { env -> + boolean willThrow = env.getSource() + if (willThrow) { + throw new RuntimeException("error") + } + return "no error" + } as DataFetcher + + def f4Fetcher = { env -> "Some Value" } as DataFetcher + def nonNullFieldFetcher = { env -> null } as DataFetcher + + def schema = TestUtil.schema(spec, + ["Query" : + [ + "f1": f1Fetcher, + "f2": f2Fetcher, + "f3": f3Fetcher, + "f4": f4Fetcher + ], + "Test" : + [ + "sub1": sub1Fetcher, + "sub2": sub2Fetcher + ], + "NonNullType": + [ + "nonNullField": nonNullFieldFetcher + ] + ]) + + + GraphQL graphQL = GraphQL.newGraphQL(schema).build() + + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query(""" + { + f1 + f2 { + sub1 + sub2 + } + f3 + f4 { + nonNullField + } + } + """) + .build() + + List errors = graphQL.execute(executionInput).getErrors() + + then: + + errors.size() == 4 + + def error = errors.get(0) as ExceptionWhileDataFetching + ["f1"] == error.getPath() + + def error2 = errors.get(1) as ExceptionWhileDataFetching + ["f2", 1, "sub2"] == error2.getPath() + + def error3 = errors.get(2) as SerializationError + ["f3"] == error3.getPath() + + def error4 = errors.get(3) as NonNullableFieldWasNullException + ["f4", "nonNullField"] == error4.getPath() + } +} diff --git a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy index 7317a6bafd..8a76002d8c 100644 --- a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy +++ b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy @@ -15,7 +15,7 @@ class NonNullableFieldValidatorTest extends Specification { NonNullableFieldValidator validator = new NonNullableFieldValidator(context, typeInfo) when: - validator.validate(null) + validator.validate(ExecutionPath.rootPath(), null) then: thrown(NonNullableFieldWasNullException) @@ -28,7 +28,7 @@ class NonNullableFieldValidatorTest extends Specification { NonNullableFieldValidator validator = new NonNullableFieldValidator(context, typeInfo) when: - def result = validator.validate(null) + def result = validator.validate(ExecutionPath.rootPath(), null) then: result == null