From db24c8f62b760b1483845ded26ebb07e6138b167 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 18 Mar 2022 08:35:11 +0000 Subject: [PATCH] Replace GraphQLError with GraphQlResponseError This allows exposing additional conveniences for inspecting errors. See gh-10 --- .../test/tester/AbstractDirectTransport.java | 4 +- .../tester/AbstractGraphQlTesterBuilder.java | 6 +- .../test/tester/DefaultGraphQlTester.java | 28 ++-- .../graphql/test/tester/GraphQlTester.java | 10 +- .../graphql/GraphQlResponse.java | 4 +- .../graphql/GraphQlResponseError.java | 66 ++++++++ .../graphql/RequestOutput.java | 48 +++++- .../client/DefaultClientGraphQlResponse.java | 19 +-- .../graphql/client/MapGraphQlError.java | 137 ---------------- .../graphql/client/MapGraphQlResponse.java | 147 ++++++++++++++++-- .../graphql/client/ResponseField.java | 7 +- .../client/SubscriptionErrorException.java | 9 +- .../client/WebSocketGraphQlTransport.java | 6 +- .../client/MapGraphQlResponseTests.java | 11 +- .../MockWebSocketGraphQlTransportTests.java | 8 +- 15 files changed, 297 insertions(+), 213 deletions(-) create mode 100644 spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponseError.java delete mode 100644 spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlError.java diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractDirectTransport.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractDirectTransport.java index a0b29e3f..49feeb6d 100644 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractDirectTransport.java +++ b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractDirectTransport.java @@ -19,13 +19,13 @@ package org.springframework.graphql.test.tester; import java.util.List; import graphql.ExecutionResult; -import graphql.GraphQLError; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.graphql.RequestOutput; import org.springframework.graphql.client.GraphQlTransport; import org.springframework.test.util.AssertionErrors; @@ -58,7 +58,7 @@ abstract class AbstractDirectTransport implements GraphQlTransport { Object data = output.getData(); AssertionErrors.assertTrue("Not a Publisher: " + data, data instanceof Publisher); - List errors = output.getErrors(); + List errors = output.getErrors(); AssertionErrors.assertTrue("Subscription errors: " + errors, CollectionUtils.isEmpty(errors)); return Flux.from((Publisher) data) diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractGraphQlTesterBuilder.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractGraphQlTesterBuilder.java index 85316663..b326cd6c 100644 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractGraphQlTesterBuilder.java +++ b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/AbstractGraphQlTesterBuilder.java @@ -24,8 +24,8 @@ import com.jayway.jsonpath.Configuration; import com.jayway.jsonpath.spi.json.JacksonJsonProvider; import com.jayway.jsonpath.spi.mapper.JacksonMappingProvider; import com.jayway.jsonpath.spi.mapper.MappingProvider; -import graphql.GraphQLError; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.graphql.client.AbstractGraphQlClientBuilder; import org.springframework.graphql.client.GraphQlTransport; import org.springframework.graphql.support.CachingDocumentSource; @@ -57,7 +57,7 @@ public abstract class AbstractGraphQlTesterBuilder errorFilter; + private Predicate errorFilter; private DocumentSource documentSource = new CachingDocumentSource(new ResourceDocumentSource()); @@ -67,7 +67,7 @@ public abstract class AbstractGraphQlTesterBuilder predicate) { + public B errorFilter(Predicate predicate) { this.errorFilter = (this.errorFilter != null ? errorFilter.and(predicate) : predicate); return self(); } diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java index 1628b1ae..db7356fc 100644 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java +++ b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java @@ -31,12 +31,12 @@ import com.jayway.jsonpath.Configuration; import com.jayway.jsonpath.DocumentContext; import com.jayway.jsonpath.JsonPath; import com.jayway.jsonpath.TypeRef; -import graphql.GraphQLError; import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.ResolvableType; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.graphql.client.GraphQlTransport; import org.springframework.graphql.support.DocumentSource; import org.springframework.lang.Nullable; @@ -61,7 +61,7 @@ final class DefaultGraphQlTester implements GraphQlTester { private final GraphQlTransport transport; @Nullable - private final Predicate errorFilter; + private final Predicate errorFilter; private final Configuration jsonPathConfig; @@ -76,7 +76,7 @@ final class DefaultGraphQlTester implements GraphQlTester { * Package private constructor for use from {@link AbstractGraphQlTesterBuilder}. */ DefaultGraphQlTester( - GraphQlTransport transport, @Nullable Predicate errorFilter, + GraphQlTransport transport, @Nullable Predicate errorFilter, Configuration jsonPathConfig, DocumentSource documentSource, Duration timeout, Consumer> builderInitializer) { @@ -209,15 +209,15 @@ final class DefaultGraphQlTester implements GraphQlTester { private final Supplier jsonContent; - private final List errors; + private final List errors; - private final List unexpectedErrors; + private final List unexpectedErrors; private final Consumer assertDecorator; private ResponseDelegate( - GraphQlResponse response, @Nullable Predicate errorFilter, + GraphQlResponse response, @Nullable Predicate errorFilter, Consumer assertDecorator, Configuration jsonPathConfig) { this.jsonDoc = JsonPath.parse(response.toMap(), jsonPathConfig); @@ -253,9 +253,9 @@ final class DefaultGraphQlTester implements GraphQlTester { this.assertDecorator.accept(task); } - boolean filterErrors(Predicate predicate) { + boolean filterErrors(Predicate predicate) { boolean filtered = false; - for (GraphQLError error : this.errors) { + for (GraphQlResponseError error : this.errors) { if (predicate.test(error)) { this.unexpectedErrors.remove(error); filtered = true; @@ -264,12 +264,12 @@ final class DefaultGraphQlTester implements GraphQlTester { return filtered; } - void expectErrors(Predicate predicate) { + void expectErrors(Predicate predicate) { boolean filtered = filterErrors(predicate); this.assertDecorator.accept(() -> AssertionErrors.assertTrue("No matching errors.", filtered)); } - void consumeErrors(Consumer> consumer) { + void consumeErrors(Consumer> consumer) { filterErrors(error -> true); consumer.accept(this.errors); } @@ -293,7 +293,7 @@ final class DefaultGraphQlTester implements GraphQlTester { private final ResponseDelegate delegate; private DefaultResponse( - GraphQlResponse response, @Nullable Predicate errorFilter, + GraphQlResponse response, @Nullable Predicate errorFilter, Consumer assertDecorator, Configuration jsonPathConfig) { this.delegate = new ResponseDelegate(response, errorFilter, assertDecorator, jsonPathConfig); @@ -311,13 +311,13 @@ final class DefaultGraphQlTester implements GraphQlTester { } @Override - public Errors filter(Predicate predicate) { + public Errors filter(Predicate predicate) { this.delegate.filterErrors(predicate); return this; } @Override - public Errors expect(Predicate predicate) { + public Errors expect(Predicate predicate) { this.delegate.expectErrors(predicate); return this; } @@ -329,7 +329,7 @@ final class DefaultGraphQlTester implements GraphQlTester { } @Override - public Traversable satisfy(Consumer> consumer) { + public Traversable satisfy(Consumer> consumer) { this.delegate.consumeErrors(consumer); return this; } diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/GraphQlTester.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/GraphQlTester.java index 392ed9b7..1e4eb028 100644 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/GraphQlTester.java +++ b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/GraphQlTester.java @@ -21,10 +21,10 @@ import java.util.List; import java.util.function.Consumer; import java.util.function.Predicate; -import graphql.GraphQLError; import reactor.core.publisher.Flux; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.graphql.client.GraphQlTransport; import org.springframework.graphql.support.DocumentSource; import org.springframework.graphql.support.ResourceDocumentSource; @@ -103,7 +103,7 @@ public interface GraphQlTester { * @param predicate the error filter to add * @return the same builder instance */ - B errorFilter(Predicate predicate); + B errorFilter(Predicate predicate); /** * Configure a {@link DocumentSource} for use with @@ -448,7 +448,7 @@ public interface GraphQlTester { * @param errorPredicate the error filter to add * @return the same spec to add more filters before {@link #verify()} */ - Errors filter(Predicate errorPredicate); + Errors filter(Predicate errorPredicate); /** * Use this to declare errors that are expected. @@ -461,7 +461,7 @@ public interface GraphQlTester { * @param errorPredicate the predicate for the expected error * @return the same spec to add more filters or expected errors */ - Errors expect(Predicate errorPredicate); + Errors expect(Predicate errorPredicate); /** * Verify there are either no errors or that there no unexpected errors that have @@ -477,7 +477,7 @@ public interface GraphQlTester { * @param errorsConsumer to inspect errors with * @return a spec to switch to a data path */ - Traversable satisfy(Consumer> errorsConsumer); + Traversable satisfy(Consumer> errorsConsumer); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponse.java b/spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponse.java index 19a1a5b8..18d46d2a 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponse.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponse.java @@ -20,8 +20,6 @@ package org.springframework.graphql; import java.util.List; import java.util.Map; -import graphql.GraphQLError; - import org.springframework.lang.Nullable; /** @@ -61,7 +59,7 @@ public interface GraphQlResponse { * response is not {@link #isValid() valid} and/or "field errors" for a * partial response. */ - List getErrors(); + List getErrors(); /** * Return implementor specific, protocol extensions, if any. diff --git a/spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponseError.java b/spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponseError.java new file mode 100644 index 00000000..06612893 --- /dev/null +++ b/spring-graphql/src/main/java/org/springframework/graphql/GraphQlResponseError.java @@ -0,0 +1,66 @@ +package org.springframework.graphql; + +import java.util.List; +import java.util.Map; + +import graphql.ErrorClassification; +import graphql.language.SourceLocation; + +import org.springframework.lang.Nullable; + +/** + * Represents a GraphQL response error. + * + * @author Rossen Stoyanchev + * @since 1.0 + */ +public interface GraphQlResponseError { + + /** + * Return the message with a description of the error intended for the + * developer as a guide to understand and correct the error. + */ + @Nullable + String getMessage(); + + /** + * Return a list of locations in the GraphQL document, if the error can be + * associated to a particular point in the document. Each location has a + * line and a column, both positive, starting from 1 and describing the + * beginning of an associated syntax element. + */ + List getLocations(); + + /** + * Return a classification for the error that is specific to GraphQL Java. + * This is serialized under {@link #getExtensions() "extensions"} in the + * response map. + * @see graphql.ErrorType + * @see org.springframework.graphql.execution.ErrorType + */ + @Nullable + ErrorClassification getErrorType(); + + /** + * Return the path to a response field which experienced the error, + * if the error can be associated to a particular field in the result. This + * allows a client to identify whether a {@code null} result is intentional + * or caused by an error. + *

This list contains path segments starting at the root of the response + * and ending with the field associated with the error. Path segments that + * represent fields are strings, and path segments that represent list + * indices are 0-indexed integers. If the error happens in an aliased field, + * the path uses the aliased name, since it represents a path in the + * response, not in the request. + */ + @Nullable + List getPath(); + + /** + * Return a map with GraphQL Java specific error details such as the + * {@link #getErrorType()}. + */ + @Nullable + Map getExtensions(); + +} diff --git a/spring-graphql/src/main/java/org/springframework/graphql/RequestOutput.java b/spring-graphql/src/main/java/org/springframework/graphql/RequestOutput.java index e8680a64..349edb6e 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/RequestOutput.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/RequestOutput.java @@ -18,10 +18,13 @@ package org.springframework.graphql; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import graphql.ErrorClassification; import graphql.ExecutionInput; import graphql.ExecutionResult; import graphql.GraphQLError; +import graphql.language.SourceLocation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -55,9 +58,7 @@ public class RequestOutput implements GraphQlResponse { * Constructor to re-wrap from transport specific subclass. */ protected RequestOutput(RequestOutput requestOutput) { - Assert.notNull(requestOutput, "RequestOutput is required."); - this.input = requestOutput.getExecutionInput(); - this.result = requestOutput.result; + this(requestOutput.getExecutionInput(), requestOutput.result); } @@ -84,8 +85,8 @@ public class RequestOutput implements GraphQlResponse { return this.result.getData(); } - public List getErrors() { - return this.result.getErrors(); + public List getErrors() { + return this.result.getErrors().stream().map(OutputError::new).collect(Collectors.toList()); } public Map getExtensions() { @@ -102,4 +103,41 @@ public class RequestOutput implements GraphQlResponse { return this.result.toString(); } + + private static class OutputError implements GraphQlResponseError { + + private final GraphQLError delegate; + + OutputError(GraphQLError delegate) { + this.delegate = delegate; + } + + @Override + public String getMessage() { + return this.delegate.getMessage(); + } + + @Override + public List getLocations() { + return this.delegate.getLocations(); + } + + @Override + public ErrorClassification getErrorType() { + return this.delegate.getErrorType(); + } + + @Override + public List getPath() { + return this.delegate.getPath(); + } + + @Override + public Map getExtensions() { + return this.delegate.getExtensions(); + } + + } + + } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientGraphQlResponse.java b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientGraphQlResponse.java index c97403c4..acb932c0 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientGraphQlResponse.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientGraphQlResponse.java @@ -20,8 +20,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import graphql.GraphQLError; - import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.ResolvableType; import org.springframework.core.codec.Decoder; @@ -31,7 +29,9 @@ import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import org.springframework.util.MimeType; import org.springframework.util.MimeTypeUtils; @@ -54,7 +54,7 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C DefaultClientGraphQlResponse( GraphQlRequest request, GraphQlResponse response, Encoder encoder, Decoder decoder) { - super(response.toMap()); + super(response); this.request = request; this.encoder = encoder; @@ -72,7 +72,7 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C List dataPath = parseFieldPath(path); Object value = getFieldValue(dataPath); - List errors = getFieldErrors(dataPath); + List errors = getFieldErrors(dataPath); return new DefaultField(path, dataPath, (value != NO_VALUE ? value : null), errors); } @@ -97,13 +97,13 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C private final List parsedPath; - private final List errors; + private final List errors; @Nullable private final Object value; public DefaultField( - String path, List parsedPath, @Nullable Object value, List errors) { + String path, List parsedPath, @Nullable Object value, List errors) { this.path = path; this.parsedPath = parsedPath; @@ -128,8 +128,9 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C } @Override - public GraphQLError getError() { - for (GraphQLError error : this.errors) { + public GraphQlResponseError getError() { + for (GraphQlResponseError error : this.errors) { + Assert.notNull(error.getPath(), "Expected field error"); if (error.getPath().size() <= this.parsedPath.size()) { return error; } @@ -138,7 +139,7 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C } @Override - public List getErrors() { + public List getErrors() { return this.errors; } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlError.java b/spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlError.java deleted file mode 100644 index 75617eeb..00000000 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlError.java +++ /dev/null @@ -1,137 +0,0 @@ -/* - * Copyright 2002-2022 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.graphql.client; - -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; - -import graphql.ErrorClassification; -import graphql.GraphQLError; -import graphql.GraphqlErrorHelper; -import graphql.language.SourceLocation; - -import org.springframework.graphql.execution.ErrorType; -import org.springframework.lang.Nullable; -import org.springframework.util.Assert; - -/** - * {@link GraphQLError} that wraps a deserialized the GraphQL response map. - * - * @author Rossen Stoyanchev - * @since 1.0.0 - */ -@SuppressWarnings("serial") -final class MapGraphQlError implements GraphQLError { - - private final Map errorMap; - - private final List locations; - - - MapGraphQlError(Map errorMap) { - Assert.notNull(errorMap, "'errorMap' is required"); - this.errorMap = errorMap; - this.locations = initLocations(errorMap); - } - - @SuppressWarnings("unchecked") - private static List initLocations(Map errorMap) { - List> locations = (List>) errorMap.get("locations"); - if (locations == null) { - return Collections.emptyList(); - } - return locations.stream() - .map(map -> new SourceLocation( - (int) map.getOrDefault("line", 0), - (int) map.getOrDefault("column", 0), - (String) map.get("sourceName"))) - .collect(Collectors.toList()); - } - - - @Override - @Nullable - public String getMessage() { - return (String) errorMap.get("message"); - } - - @Override - public List getLocations() { - return this.locations; - } - - @Override - @Nullable - public ErrorClassification getErrorType() { - // Attempt the reverse of how errorType is serialized in GraphqlErrorHelper.toSpecification. - // However, we can only do that for ErrorClassification enums that we know of. - String value = (getExtensions() != null ? (String) getExtensions().get("classification") : null); - if (value != null) { - try { - return graphql.ErrorType.valueOf(value); - } - catch (IllegalArgumentException ex) { - // ignore - } - try { - return ErrorType.valueOf(value); - } - catch (IllegalArgumentException ex) { - // ignore - } - } - return null; - } - - @SuppressWarnings("unchecked") - @Override - @Nullable - public List getPath() { - return (List) this.errorMap.get("path"); - } - - @SuppressWarnings("unchecked") - @Override - @Nullable - public Map getExtensions() { - return (Map) this.errorMap.get("extensions"); - } - - @Override - public Map toSpecification() { - return GraphqlErrorHelper.toSpecification(this); - } - - @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") - @Override - public boolean equals(Object other) { - return GraphqlErrorHelper.equals(this, other); - } - - @Override - public int hashCode() { - return GraphqlErrorHelper.hashCode(this); - } - - @Override - public String toString() { - return toSpecification().toString(); - } - -} diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlResponse.java b/spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlResponse.java index 9eb17db9..a9b578b0 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlResponse.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/MapGraphQlResponse.java @@ -20,13 +20,19 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import graphql.ErrorClassification; import graphql.GraphQLError; +import graphql.language.SourceLocation; import org.springframework.graphql.GraphQlResponse; +import org.springframework.graphql.GraphQlResponseError; +import org.springframework.graphql.execution.ErrorType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -45,7 +51,7 @@ class MapGraphQlResponse implements GraphQlResponse { private final Map responseMap; - private final List errors; + private final List errors; MapGraphQlResponse(Map responseMap) { @@ -54,17 +60,17 @@ class MapGraphQlResponse implements GraphQlResponse { this.errors = wrapErrors(responseMap); } + MapGraphQlResponse(GraphQlResponse response) { + Assert.notNull(response, "'GraphQlResponse' is required"); + this.responseMap = response.toMap(); + this.errors = response.getErrors(); + } + @SuppressWarnings("unchecked") - private static List wrapErrors(Map responseMap) { - List> rawErrors = (List>) responseMap.get("errors"); - if (CollectionUtils.isEmpty(rawErrors)) { - return Collections.emptyList(); - } - List errors = new ArrayList<>(rawErrors.size()); - for (Map map : rawErrors) { - errors.add(new MapGraphQlError(map)); - } - return errors; + private static List wrapErrors(Map map) { + List> errors = (List>) map.get("errors"); + errors = (errors != null ? errors : Collections.emptyList()); + return errors.stream().map(MapError::new).collect(Collectors.toList()); } @@ -74,7 +80,7 @@ class MapGraphQlResponse implements GraphQlResponse { } @Override - public List getErrors() { + public List getErrors() { return this.errors; } @@ -177,12 +183,12 @@ class MapGraphQlResponse implements GraphQlResponse { * @param fieldPath the field path to match * @return errors whose path starts with the dataPath */ - protected List getFieldErrors(List fieldPath) { + protected List getFieldErrors(List fieldPath) { if (fieldPath.isEmpty()) { return Collections.emptyList(); } - List fieldErrors = Collections.emptyList(); - for (GraphQLError error : this.errors) { + List fieldErrors = Collections.emptyList(); + for (GraphQlResponseError error : this.errors) { List errorPath = error.getPath(); if (CollectionUtils.isEmpty(errorPath)) { continue; @@ -217,4 +223,115 @@ class MapGraphQlResponse implements GraphQlResponse { return this.responseMap.toString(); } + + /** + * {@link GraphQLError} that wraps a deserialized the GraphQL response map. + */ + @SuppressWarnings("serial") + private static final class MapError implements GraphQlResponseError { + + private final Map errorMap; + + private final List locations; + + MapError(Map errorMap) { + Assert.notNull(errorMap, "'errorMap' is required"); + this.errorMap = errorMap; + this.locations = initLocations(errorMap); + } + + @SuppressWarnings("unchecked") + private static List initLocations(Map errorMap) { + List> locations = (List>) errorMap.get("locations"); + if (locations == null) { + return Collections.emptyList(); + } + return locations.stream() + .map(map -> new SourceLocation( + (int) map.getOrDefault("line", 0), + (int) map.getOrDefault("column", 0), + (String) map.get("sourceName"))) + .collect(Collectors.toList()); + } + + @Override + @Nullable + public String getMessage() { + return (String) errorMap.get("message"); + } + + @Override + public List getLocations() { + return this.locations; + } + + @Override + @Nullable + public ErrorClassification getErrorType() { + // Attempt the reverse of how errorType is serialized in GraphqlErrorHelper.toSpecification. + // However, we can only do that for ErrorClassification enums that we know of. + String value = (getExtensions() != null ? (String) getExtensions().get("classification") : null); + if (value != null) { + try { + return graphql.ErrorType.valueOf(value); + } + catch (IllegalArgumentException ex) { + // ignore + } + try { + return ErrorType.valueOf(value); + } + catch (IllegalArgumentException ex) { + // ignore + } + } + return null; + } + + @SuppressWarnings("unchecked") + @Override + @Nullable + public List getPath() { + return (List) this.errorMap.get("path"); + } + + @SuppressWarnings("unchecked") + @Override + @Nullable + public Map getExtensions() { + return (Map) this.errorMap.get("extensions"); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || this.getClass() != o.getClass()) { + return false; + } + GraphQlResponseError other = (GraphQlResponseError) o; + return (ObjectUtils.nullSafeEquals(getMessage(), other.getMessage()) && + ObjectUtils.nullSafeEquals(getLocations(), other.getLocations()) && + ObjectUtils.nullSafeEquals(getPath(), other.getPath()) && + getErrorType() == other.getErrorType()); + } + + @Override + public int hashCode() { + int result = 1; + result = 31 * result + ObjectUtils.nullSafeHashCode(getMessage()); + result = 31 * result + ObjectUtils.nullSafeHashCode(getLocations()); + result = 31 * result + ObjectUtils.nullSafeHashCode(getPath()); + result = 31 * result + ObjectUtils.nullSafeHashCode(getErrorType()); + return result; + } + + @Override + public String toString() { + return this.errorMap.toString(); + } + + } + } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java b/spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java index 51acf86d..64f7aaa5 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java @@ -19,9 +19,8 @@ package org.springframework.graphql.client; import java.util.List; -import graphql.GraphQLError; - import org.springframework.core.ParameterizedTypeReference; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.lang.Nullable; /** @@ -72,7 +71,7 @@ public interface ResponseField { * error with the same path as the field path */ @Nullable - GraphQLError getError(); + GraphQlResponseError getError(); /** * Return all field errors including errors above, at, and below this field. @@ -80,7 +79,7 @@ public interface ResponseField { * below. When the field does not have a value, there is only one error, and * it is the same as {@link #getError()}. */ - List getErrors(); + List getErrors(); /** * Decode the field to an entity of the given type. diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/SubscriptionErrorException.java b/spring-graphql/src/main/java/org/springframework/graphql/client/SubscriptionErrorException.java index b83654d2..f17c0b84 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/SubscriptionErrorException.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/SubscriptionErrorException.java @@ -18,9 +18,8 @@ package org.springframework.graphql.client; import java.util.List; -import graphql.GraphQLError; - import org.springframework.graphql.GraphQlRequest; +import org.springframework.graphql.GraphQlResponseError; /** * WebSocket {@link GraphQlTransportException} raised when a subscription @@ -33,14 +32,14 @@ import org.springframework.graphql.GraphQlRequest; @SuppressWarnings("serial") public class SubscriptionErrorException extends GraphQlTransportException { - private final List errors; + private final List errors; /** * Constructor with the request details and the errors listed in the payload * of the {@code "errors"} message. */ - public SubscriptionErrorException(GraphQlRequest request, List errors) { + public SubscriptionErrorException(GraphQlRequest request, List errors) { super("GraphQL subscription completed with an \"error\" message, " + "with the following errors: " + errors, null, request); this.errors = errors; @@ -50,7 +49,7 @@ public class SubscriptionErrorException extends GraphQlTransportException { /** * Return the errors contained in the GraphQL over WebSocket "errors" message. */ - public List getErrors() { + public List getErrors() { return this.errors; } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/WebSocketGraphQlTransport.java b/spring-graphql/src/main/java/org/springframework/graphql/client/WebSocketGraphQlTransport.java index 84c2b55d..127ba608 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/WebSocketGraphQlTransport.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/WebSocketGraphQlTransport.java @@ -24,7 +24,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; -import graphql.GraphQLError; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.Scannable; @@ -34,6 +33,7 @@ import reactor.core.publisher.Sinks; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.graphql.web.support.GraphQlMessage; import org.springframework.graphql.web.support.GraphQlMessageType; import org.springframework.http.HttpHeaders; @@ -514,8 +514,8 @@ final class WebSocketGraphQlTransport implements GraphQlTransport { emitResult = responseState.sink().tryEmitValue(response); } else { - List graphQLErrors = response.getErrors(); - Exception ex = new SubscriptionErrorException(subscriptionState.request(), graphQLErrors); + List errors = response.getErrors(); + Exception ex = new SubscriptionErrorException(subscriptionState.request(), errors); emitResult = subscriptionState.sink().tryEmitError(ex); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/client/MapGraphQlResponseTests.java b/spring-graphql/src/test/java/org/springframework/graphql/client/MapGraphQlResponseTests.java index 16b36223..d02b0f6d 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/client/MapGraphQlResponseTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/client/MapGraphQlResponseTests.java @@ -29,6 +29,7 @@ import graphql.execution.ResultPath; import org.junit.jupiter.api.Test; import org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.lang.Nullable; import static org.assertj.core.api.Assertions.assertThat; @@ -140,9 +141,12 @@ public class MapGraphQlResponseTests { .map(GraphQLError::toSpecification).collect(Collectors.toList()); MapGraphQlResponse response = new MapGraphQlResponse(Collections.singletonMap("errors", errorList)); - List errors = response.getFieldErrors(path); + List errors = response.getFieldErrors(path); - assertThat(errors).containsExactly(error1, error2, error3); + assertThat(errors).hasSize(3); + assertThat(errors.get(0).getPath()).containsExactly("me"); + assertThat(errors.get(1).getPath()).containsExactly("me", "friends"); + assertThat(errors.get(2).getPath()).containsExactly("me", "friends", 0, "name"); } private GraphQLError createError(@Nullable String errorPath, String message) { @@ -150,8 +154,7 @@ public class MapGraphQlResponseTests { if (errorPath != null) { builder = builder.path(ResultPath.parse(errorPath)); } - Map errorMap = builder.build().toSpecification(); - return new MapGraphQlError(errorMap); + return builder.build(); } } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/client/MockWebSocketGraphQlTransportTests.java b/spring-graphql/src/test/java/org/springframework/graphql/client/MockWebSocketGraphQlTransportTests.java index ea41b5d7..d2efbab2 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/client/MockWebSocketGraphQlTransportTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/client/MockWebSocketGraphQlTransportTests.java @@ -25,7 +25,6 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; -import graphql.GraphQLError; import graphql.GraphqlErrorBuilder; import org.junit.jupiter.api.Test; import reactor.core.publisher.Flux; @@ -34,6 +33,7 @@ import reactor.test.StepVerifier; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; +import org.springframework.graphql.GraphQlResponseError; import org.springframework.graphql.web.TestWebSocketClient; import org.springframework.graphql.web.TestWebSocketConnection; import org.springframework.graphql.web.support.GraphQlMessage; @@ -110,7 +110,7 @@ public class MockWebSocketGraphQlTransportTests { StepVerifier.create(this.transport.execute(request)) .consumeNextWith(result -> { assertThat(result.isValid()).isFalse(); - assertThat(result.getErrors()).extracting(GraphQLError::getMessage).containsExactly("boo"); + assertThat(result.getErrors()).extracting(GraphQlResponseError::getMessage).containsExactly("boo"); }) .expectComplete() .verify(TIMEOUT); @@ -128,8 +128,8 @@ public class MockWebSocketGraphQlTransportTests { StepVerifier.create(this.transport.executeSubscription(request)) .expectNext(this.response1) .expectErrorSatisfies(actualEx -> { - List errorList = ((SubscriptionErrorException) actualEx).getErrors(); - assertThat(errorList).extracting(GraphQLError::getMessage).containsExactly("boo"); + List errors = ((SubscriptionErrorException) actualEx).getErrors(); + assertThat(errors).extracting(GraphQlResponseError::getMessage).containsExactly("boo"); }) .verify(TIMEOUT);