From cdd8b67fedd3c741a4c4a8db25882ecd1c6eadbe Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 18 Mar 2022 17:21:06 +0000 Subject: [PATCH] Refine GraphQlResponseField getError() Instead of a simple check, looking for an associated field error at or above the field, this method now more focused on finding the reason for a failure when the field has no value. This allows performing a more thorough search including cases when the field error is at, above, or even below (e.g. non-null nested field that bubbled up), in the end falling back on request errors (e.g. failed response without any field errors). Also, rename ResponseField to GraphQlResponseField and move to a top-level class. See gh-10 --- .../src/docs/asciidoc/client.adoc | 93 +++++------ .../graphql/client/ClientGraphQlResponse.java | 8 +- .../client/DefaultClientGraphQlResponse.java | 131 +++------------- .../graphql/client/DefaultGraphQlClient.java | 18 +-- .../client/DefaultGraphQlResponseField.java | 144 ++++++++++++++++++ .../graphql/client/FieldAccessException.java | 13 +- .../graphql/client/GraphQlClient.java | 34 +++-- ...seField.java => GraphQlResponseField.java} | 45 +++--- .../DefaultGraphQlClientResponseTests.java | 6 +- .../graphql/client/GraphQlClientTests.java | 6 +- 10 files changed, 284 insertions(+), 214 deletions(-) create mode 100644 spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlResponseField.java rename spring-graphql/src/main/java/org/springframework/graphql/client/{ResponseField.java => GraphQlResponseField.java} (63%) diff --git a/spring-graphql-docs/src/docs/asciidoc/client.adoc b/spring-graphql-docs/src/docs/asciidoc/client.adoc index 3a47aeb9..b1a0eeb0 100644 --- a/spring-graphql-docs/src/docs/asciidoc/client.adoc +++ b/spring-graphql-docs/src/docs/asciidoc/client.adoc @@ -135,7 +135,7 @@ which is a strategy for loading the document for a request by file name. Once you have a <>, you can begin to perform requests via <> or <> -where the former is merely a shortcut for the latter. +where the former is only a shortcut for the latter. @@ -159,25 +159,21 @@ The below retrieves and decodes the data for a query: .retrieve("project") <2> .toEntity(Project.class); <3> ---- -<1> The operation to perform -<2> Specify a path under the "data" key in the response map -<3> Decode the data at the path to the target type +<1> The operation to perform. +<2> The path under the "data" key in the response map to decode from. +<3> Decode the data at the path to the target type. -The document is a `String` that could be a literal or produced through a code generated -request object. You can also define documents in files and use a +The input document is a `String` that could be a literal or produced through a code +generated request object. You can also define documents in files and use a <> to resole them by file name. The path is relative to the "data" key and uses a simple dot (".") separated notation -for nested fields with optional array indices for list elements, e.g. `"project.name"`, -`"project.releases[0].version"`, and so on. +for nested fields with optional array indices for list elements, e.g. `"project.name"` +or `"project.releases[0].version"`. -Decoding can fail with `FieldAccessException` if the given path is not present in the -response map, or when the value is `null` and there is an error for the field. - -By default, `FieldAccessException` is also raised on `retrieve` for partial data where -the field value exists but nested fields may be `null` with a field error. In such -cases, you can handle the exception to examine the errors and decide whether or how to -decode the partial data: +Decoding can result in `FieldAccessException` if the given path is not present, or the +field value is `null` and has an error. `FieldAccessException` provides access to the +response and the field: [source,java,indent=0,subs="verbatim,quotes"] ---- @@ -185,24 +181,20 @@ decode the partial data: .retrieve("project") .toEntity(Project.class) .onErrorResume(FieldAccessException.class, ex -> { - ResponseField field = ex.getField(); - // Use field to check nested field errors and/or decode - // Return Mono with Project or an error + ClientGraphQlResponse response = ex.getResponse(); + // ... + GraphQlResponseField field = ex.getField(); + // ... }); ---- -TIP: The GraphQL spec considers a partial response or a partial field to be valid, and -it may be feasible to decode them. By contrast, a failed field (i.e. value not present or -is `null` with field error) or a failed response (no "data" key) are not valid and -attempts to decode those are always rejected. - [[client-requests-execute]] === Execute -The `retrieve` method is only a shortcut to decode from a single path to a higher level -object. For more control and access to the response, use the `execute` method. +`retrieve` is only a shortcut to decode from a single path in the response map. For more +control, use the `execute` method and handle the response: For example: @@ -212,17 +204,27 @@ For example: Mono projectMono = graphQlClient.document(document) .execute() .map(response -> { - // Check response.isValid(), getErrors() + if (!response.isValid()) { + // Request failure... <1> + } ResponseField field = response.field("project"); - // Check field.hasValue(), getError() + if (!field.hasValue()) { + if (field.getError() != null) { + // Field failure... <2> + } + else { + // Optional field set to null... <3> + } + } - return field.toEntity(Project.class) + return field.toEntity(Project.class); <4> }); ---- - -You can use `execute` to check response errors, obtain different fields, check their -field errors and nested field errors, and/or decode their values. +<1> The response does not have data, only errors +<2> Field that is `null` and has an associated error +<3> Field that was set to `null` by its `DataFetcher` +<4> Decode the data at the given path @@ -272,7 +274,7 @@ You can use the `GraphQlClient` <> to customize th == Subscriptions For a subscription operation, call `retrieveSubscription` instead of `retrieve` to -obtain a stream of responses rather than a single response: +obtain a stream of responses, each decoded to a target object: [source,java,indent=0,subs="verbatim,quotes"] ---- @@ -281,29 +283,34 @@ obtain a stream of responses rather than a single response: .toEntity(String.class); ---- -Similar to the <> vs <> choice -for requests with a single response, the same choice is also available for subscriptions. -For example, for more control over each response, use `executeSubscription` instead of -`retrieveSubscription`: +Similar to the <> vs <> +alternatives for single response requests, the same is also available for subscriptions. +For more control over each response, use `executeSubscription`: [source,java,indent=0,subs="verbatim,quotes"] ---- Flux greetingFlux = client.document("subscription { greetings }") .executeSubscription() .map(response -> { - // Check response.isValid(), getErrors() + if (!response.isValid()) { + // Request failure... + } - ResponseField field = response.field("greeting"); - // Check field.isValid(), getError() + ResponseField field = response.field("project"); + if (!field.hasValue()) { + if (field.getError() != null) { + // Field failure... + } + else { + // Optional field set to null... <3> + } + } return field.toEntity(String.class) }); ---- - - -Subscriptions are supported only with the <> extension. +NOTE: Subscriptions are supported only over <>. diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/ClientGraphQlResponse.java b/spring-graphql/src/main/java/org/springframework/graphql/client/ClientGraphQlResponse.java index 428a8098..ffdaca98 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/ClientGraphQlResponse.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/ClientGraphQlResponse.java @@ -31,7 +31,7 @@ import org.springframework.graphql.GraphQlResponse; public interface ClientGraphQlResponse extends GraphQlResponse { /** - * Return the request associated with this response. + * Return the request for the response. */ GraphQlRequest getRequest(); @@ -48,10 +48,10 @@ public interface ClientGraphQlResponse extends GraphQlResponse { * * @param path relative to the "data" key * @return representation for the field with further options to inspect or - * decode its value; use {@link ResponseField#hasValue()} to check if the - * field actually exists and has a value. + * decode its value; use {@link GraphQlResponseField#hasValue()} to check if + * the field actually exists and has a value. */ - ResponseField field(String path); + GraphQlResponseField field(String path); /** * Decode the full response map to the given target type. 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 8aa7028a..8bbec4cd 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 @@ -23,19 +23,13 @@ import java.util.Map; import java.util.stream.Collectors; import org.springframework.core.ParameterizedTypeReference; -import org.springframework.core.ResolvableType; import org.springframework.core.codec.Decoder; import org.springframework.core.codec.Encoder; -import org.springframework.core.io.buffer.DataBuffer; -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; import org.springframework.util.StringUtils; @@ -70,13 +64,22 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C return this.request; } - @Override - public ResponseField field(String path) { - List dataPath = parseFieldPath(path); - return new DefaultField(path, dataPath, getFieldValue(dataPath), getFieldErrors(path)); + Encoder getEncoder() { + return this.encoder; } - private static List parseFieldPath(String path) { + Decoder getDecoder() { + return this.decoder; + } + + + @Override + public GraphQlResponseField field(String path) { + List parsedPath = parsePath(path); + return new DefaultGraphQlResponseField(this, path, parsedPath, getValue(parsedPath), getFieldErrors(path)); + } + + private static List parsePath(String path) { if (!StringUtils.hasText(path)) { return Collections.emptyList(); } @@ -117,18 +120,18 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C } @Nullable - private Object getFieldValue(List fieldPath) { + private Object getValue(List path) { Object value = (isValid() ? getData() : null); - for (Object segment : fieldPath) { + for (Object segment : path) { if (value == null) { return null; } if (segment instanceof String) { - Assert.isTrue(value instanceof Map, () -> "Invalid path " + fieldPath + ", data: " + getData()); + Assert.isTrue(value instanceof Map, () -> "Invalid path " + path + ", data: " + getData()); value = ((Map) value).getOrDefault(segment, null); } else { - Assert.isTrue(value instanceof List, () -> "Invalid path " + fieldPath + ", data: " + getData()); + Assert.isTrue(value instanceof List, () -> "Invalid path " + path + ", data: " + getData()); int index = (int) segment; value = (index < ((List) value).size() ? ((List) value).get(index) : null); } @@ -163,102 +166,4 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C return field("").toEntity(type); } - - /** - * Default implementation of {@link ResponseField}. - */ - private class DefaultField implements ResponseField { - - private final String path; - - private final List parsedPath; - - private final List fieldErrors; - - @Nullable - private final Object value; - - public DefaultField( - String path, List parsedPath, @Nullable Object value, List errors) { - - this.path = path; - this.parsedPath = parsedPath; - this.value = value; - this.fieldErrors = errors; - } - - @Override - public String getPath() { - return this.path; - } - - @Override - public List getParsedPath() { - return this.parsedPath; - } - - @Override - public boolean hasValue() { - return (this.value != null); - } - - @SuppressWarnings("unchecked") - @Override - public T getValue() { - return (T) this.value; - } - - @Override - public GraphQlResponseError getError() { - for (GraphQlResponseError error : this.fieldErrors) { - if (error.getParsedPath().size() <= this.parsedPath.size()) { - return error; - } - } - return null; - } - - @Override - public List getErrors() { - return this.fieldErrors; - } - - @Override - public D toEntity(Class entityType) { - return toEntity(ResolvableType.forType(entityType)); - } - - @Override - public D toEntity(ParameterizedTypeReference entityType) { - return toEntity(ResolvableType.forType(entityType)); - } - - @Override - public List toEntityList(Class elementType) { - return toEntity(ResolvableType.forClassWithGenerics(List.class, elementType)); - } - - @Override - public List toEntityList(ParameterizedTypeReference elementType) { - return toEntity(ResolvableType.forClassWithGenerics(List.class, ResolvableType.forType(elementType))); - } - - @SuppressWarnings({"unchecked", "ConstantConditions"}) - private T toEntity(ResolvableType targetType) { - if (this.value == null) { - throw new FieldAccessException(request, DefaultClientGraphQlResponse.this, this); - } - - DataBufferFactory bufferFactory = DefaultDataBufferFactory.sharedInstance; - MimeType mimeType = MimeTypeUtils.APPLICATION_JSON; - Map hints = Collections.emptyMap(); - - DataBuffer buffer = ((Encoder) encoder).encodeValue( - (T) this.value, bufferFactory, ResolvableType.forInstance(this.value), mimeType, hints); - - return ((Decoder) decoder).decode(buffer, targetType, mimeType, hints); - } - - } - } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java index d192a3ae..f411c0f8 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java @@ -199,14 +199,14 @@ final class DefaultGraphQlClient implements GraphQlClient { } /** - * Return the field if valid, possibly {@code null}. - * @throws FieldAccessException if the response or field is not valid + * Return the field if valid, or {@code null} if {@code null} without errors. + * @throws FieldAccessException for invalid response or failed field */ @Nullable - protected ResponseField getValidField(ClientGraphQlResponse response) { - ResponseField field = response.field(this.path); + protected GraphQlResponseField getValidField(ClientGraphQlResponse response) { + GraphQlResponseField field = response.field(this.path); if (!response.isValid() || field.getError() != null) { - throw new FieldAccessException(response.getRequest(), response, field); + throw new FieldAccessException(response, field); } return (field.hasValue() ? field : null); } @@ -236,7 +236,7 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Mono> toEntityList(Class elementType) { return this.responseMono.map(response -> { - ResponseField field = getValidField(response); + GraphQlResponseField field = getValidField(response); return (field != null ? field.toEntityList(elementType) : Collections.emptyList()); }); } @@ -244,7 +244,7 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Mono> toEntityList(ParameterizedTypeReference elementType) { return this.responseMono.map(response -> { - ResponseField field = getValidField(response); + GraphQlResponseField field = getValidField(response); return (field != null ? field.toEntityList(elementType) : Collections.emptyList()); }); } @@ -274,7 +274,7 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Flux> toEntityList(Class elementType) { return this.responseFlux.map(response -> { - ResponseField field = getValidField(response); + GraphQlResponseField field = getValidField(response); return (field != null ? field.toEntityList(elementType) : Collections.emptyList()); }); } @@ -282,7 +282,7 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Flux> toEntityList(ParameterizedTypeReference elementType) { return this.responseFlux.map(response -> { - ResponseField field = getValidField(response); + GraphQlResponseField field = getValidField(response); return (field != null ? field.toEntityList(elementType) : Collections.emptyList()); }); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlResponseField.java b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlResponseField.java new file mode 100644 index 00000000..5f6dba02 --- /dev/null +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlResponseField.java @@ -0,0 +1,144 @@ +/* + * 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 org.springframework.core.ParameterizedTypeReference; +import org.springframework.core.ResolvableType; +import org.springframework.core.codec.Decoder; +import org.springframework.core.codec.Encoder; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferFactory; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.graphql.GraphQlResponseError; +import org.springframework.lang.Nullable; +import org.springframework.util.MimeType; +import org.springframework.util.MimeTypeUtils; + + +/** + * Default implementation of {@link GraphQlResponseField}. + * + * @author Rossen Stoyanchev + * @since 1.0.0 + */ +final class DefaultGraphQlResponseField implements GraphQlResponseField { + + private final DefaultClientGraphQlResponse response; + + private final String path; + + private final List parsedPath; + + @Nullable + private final Object value; + + private final List fieldErrors; + + + DefaultGraphQlResponseField( + DefaultClientGraphQlResponse response, String path, List parsedPath, + @Nullable Object value, List errors) { + + this.response = response; + this.path = path; + this.parsedPath = parsedPath; + this.value = value; + this.fieldErrors = errors; + } + + + @Override + public String getPath() { + return this.path; + } + + @Override + public List getParsedPath() { + return this.parsedPath; + } + + @Override + public boolean hasValue() { + return (this.value != null); + } + + @SuppressWarnings("unchecked") + @Override + public T getValue() { + return (T) this.value; + } + + @Override + public GraphQlResponseError getError() { + if (!hasValue()) { + if (!this.fieldErrors.isEmpty()) { + return this.fieldErrors.get(0); + } + if (!this.response.getErrors().isEmpty()) { + return this.response.getErrors().get(0); + } + // No errors, set to null by DataFetcher + } + return null; + } + + @Override + public List getErrors() { + return this.fieldErrors; + } + + @Override + public D toEntity(Class entityType) { + return toEntity(ResolvableType.forType(entityType)); + } + + @Override + public D toEntity(ParameterizedTypeReference entityType) { + return toEntity(ResolvableType.forType(entityType)); + } + + @Override + public List toEntityList(Class elementType) { + return toEntity(ResolvableType.forClassWithGenerics(List.class, elementType)); + } + + @Override + public List toEntityList(ParameterizedTypeReference elementType) { + return toEntity(ResolvableType.forClassWithGenerics(List.class, ResolvableType.forType(elementType))); + } + + @SuppressWarnings({"unchecked", "ConstantConditions"}) + private T toEntity(ResolvableType targetType) { + if (this.value == null) { + throw new FieldAccessException(this.response, this); + } + + DataBufferFactory bufferFactory = DefaultDataBufferFactory.sharedInstance; + MimeType mimeType = MimeTypeUtils.APPLICATION_JSON; + Map hints = Collections.emptyMap(); + + DataBuffer buffer = ((Encoder) this.response.getEncoder()).encodeValue( + (T) this.value, bufferFactory, ResolvableType.forInstance(this.value), mimeType, hints); + + return ((Decoder) this.response.getDecoder()).decode(buffer, targetType, mimeType, hints); + } + +} diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/FieldAccessException.java b/spring-graphql/src/main/java/org/springframework/graphql/client/FieldAccessException.java index 6e9d13f1..95390b3e 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/FieldAccessException.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/FieldAccessException.java @@ -17,13 +17,12 @@ package org.springframework.graphql.client; -import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; /** * An exception raised on an attempt to decode data from a * {@link GraphQlResponse#isValid() failed response} or a field is not present, - * or has no value, checked via {@link ResponseField#hasValue()}. + * or has no value, checked via {@link GraphQlResponseField#hasValue()}. * * @author Rossen Stoyanchev * @since 1.0.0 @@ -33,19 +32,19 @@ public class FieldAccessException extends GraphQlClientException { private final ClientGraphQlResponse response; - private final ResponseField field; + private final GraphQlResponseField field; /** * Constructor with the request and response, and the accessed field. */ - public FieldAccessException(GraphQlRequest request, ClientGraphQlResponse response, ResponseField field) { - super(initDefaultMessage(field), null, request); + public FieldAccessException(ClientGraphQlResponse response, GraphQlResponseField field) { + super(initDefaultMessage(field), null, response.getRequest()); this.response = response; this.field = field; } - private static String initDefaultMessage(ResponseField field) { + private static String initDefaultMessage(GraphQlResponseField field) { return "Invalid field '" + field.getPath() + "', errors: " + field.getErrors(); } @@ -60,7 +59,7 @@ public class FieldAccessException extends GraphQlClientException { /** * Return the field that needed to be accessed. */ - public ResponseField getField() { + public GraphQlResponseField getField() { return this.field; } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlClient.java b/spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlClient.java index 660c16c0..b55c62da 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlClient.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlClient.java @@ -22,6 +22,7 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.graphql.GraphQlResponse; import org.springframework.graphql.support.DocumentSource; import org.springframework.graphql.support.ResourceDocumentSource; import org.springframework.lang.Nullable; @@ -180,10 +181,11 @@ public interface GraphQlClient { /** * Decode the field to an entity of the given type. * @param entityType the type to convert to - * @return {@code Mono} that provides the decoded entity, or completes - * empty when the field is {@code null} but without errors, or ends with - * a {@link FieldAccessException} if the target field is not present or - * has no value. + * @return {@code Mono} with the decoded entity. Completes empty when + * the field is {@code null} without errors, or ends with + * {@link FieldAccessException} for an invalid response or a failed field + * @see GraphQlResponse#isValid() + * @see GraphQlResponseField#getError() */ Mono toEntity(Class entityType); @@ -198,6 +200,8 @@ public interface GraphQlClient { * @return {@code Mono} with a list of decoded entities, possibly an * empty list, or ends with {@link FieldAccessException} if the target * field is not present or has no value. + * @see GraphQlResponse#isValid() + * @see GraphQlResponseField#getError() */ Mono> toEntityList(Class elementType); @@ -217,11 +221,12 @@ public interface GraphQlClient { /** * Decode the field to an entity of the given type. * @param entityType the type to convert to - * @return decoded entities, one for each response, except responses - * in which the field is {@code null} but without errors, or ending with - * {@link FieldAccessException} if the target field is not present or - * has no value in a given response; the stream may also end with a - * {@link GraphQlTransportException}. + * @return a stream of decoded entities, one for each response, excluding + * responses in which the field is {@code null} without errors. Ends with + * {@link FieldAccessException} for an invalid response or a failed field. + * May also end with a {@link GraphQlTransportException}. + * @see GraphQlResponse#isValid() + * @see GraphQlResponseField#getError() */ Flux toEntity(Class entityType); @@ -233,11 +238,12 @@ public interface GraphQlClient { /** * Decode the field to a list of entities with the given type. * @param elementType the type of elements in the list - * @return lists of decoded entities, one for each response, except responses - * in which the field is {@code null} but without errors, or ending with - * {@link FieldAccessException} if the target field is not present or - * has no value in a given response; the stream may also end with a - * {@link GraphQlTransportException}. + * @return lists of decoded entities, one for each response, excluding + * responses in which the field is {@code null} without errors. Ends with + * {@link FieldAccessException} for an invalid response or a failed field. + * May also end with a {@link GraphQlTransportException}. + * @see GraphQlResponse#isValid() + * @see GraphQlResponseField#getError() */ Flux> toEntityList(Class elementType); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java b/spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlResponseField.java similarity index 63% rename from spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java rename to spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlResponseField.java index e2a3bb58..d3d9ca44 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlResponseField.java @@ -20,6 +20,7 @@ package org.springframework.graphql.client; import java.util.List; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.graphql.GraphQlResponse; import org.springframework.graphql.GraphQlResponseError; import org.springframework.lang.Nullable; @@ -30,18 +31,15 @@ import org.springframework.lang.Nullable; * @author Rossen Stoyanchev * @since 1.0.0 */ -public interface ResponseField { +public interface GraphQlResponseField { /** * Whether the field has a value. *
    - *
  • {@code "true"} means the field is not {@code null} in which case there - * is no field {@link #getError() error}. The field may still be partial and - * have nested, field {@link #getErrors() errors}. - *
  • {@code "false"} means the field is {@code null} or does not exist. - * Check for a field {@link #getError()}, which may be on the field or on a - * parent field. The field may also be {@code null} because it is defined as - * optional in the schema. + *
  • {@code "true"} means the field is not {@code null}, and therefore valid, + * although it may be partial with nested field {@link #getErrors() errors}. + *
  • {@code "false"} means the field is {@code null} or doesn't exist; use + * {@link #getError()} to check if the field is {@code null} due to an error. *
*/ boolean hasValue(); @@ -68,13 +66,23 @@ public interface ResponseField { T getValue(); /** - * Return the error for this field, if any. The error may be for this field - * when the field is {@code null}, or it may be for a parent field, when the - * current field does not exist. - *

Note: The field error is identified by searching for - * the first error with a matching path that is shorter or the same as the - * field path. According to the GraphQL spec, section 6.4.4, - * "Handling Field Errors", there should be only one field error per field. + * Return the error that provides the reason for a failed field. + *

When the field does not {@link #hasValue() have} a + * value, this method looks for the first field error. According to the + * GraphQL spec, section 6.4.4, "Handling Field Errors", there should be + * only one error per field. The returned field error may be: + *

    + *
  • on the field + *
  • on a parent field, when the field is not present + *
  • on a nested field, when a {@code non-null} nested field error bubbles up + *
+ *

As a fallback, this method also checks "request errors" in case the + * entire response is not {@link GraphQlResponse#isValid() valid}. If there + * are no errors at all, {@code null} is returned, and it implies the field + * value was set to {@code null} by its {@code DataFetcher}. + *

When the field does have a value, it is considered + * valid and this method returns {@code null}, although the field may be + * partial and contain {@link #getErrors() errors} on nested fields. * @return return the error for this field, or {@code null} if there is no * error with the same path as the field path */ @@ -83,9 +91,10 @@ public interface ResponseField { /** * Return all field errors including errors above, at, and below this field. - *

In practice, when the field has a value, all errors are for fields - * below. When the field does not have a value, there is only one error, and - * it is the same as {@link #getError()}. + *

In practice, when the field does have a value, it is + * considered valid but possibly partial with nested field errors. When the + * field does not have a value, there should be only one + * field error, and in that case it is better to use {@link #getError()}. */ List getErrors(); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/client/DefaultGraphQlClientResponseTests.java b/spring-graphql/src/test/java/org/springframework/graphql/client/DefaultGraphQlClientResponseTests.java index cb4a5956..dcd0c3c6 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/client/DefaultGraphQlClientResponseTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/client/DefaultGraphQlClientResponseTests.java @@ -127,7 +127,7 @@ public class DefaultGraphQlClientResponseTests { GraphQLError error2 = createError("/me/friends", "fail-me-friends"); GraphQLError error3 = createError("/me/friends[0]/name", "fail-me-friends-name"); - ResponseField field = getField(path, error0, error1, error2, error3); + GraphQlResponseField field = getField(path, error0, error1, error2, error3); List errors = field.getErrors(); assertThat(errors).hasSize(3); @@ -144,13 +144,13 @@ public class DefaultGraphQlClientResponseTests { return builder.build(); } - private ResponseField getField(String path, String dataJson) throws Exception { + private GraphQlResponseField getField(String path, String dataJson) throws Exception { Map dataMap = mapper.readValue(dataJson, Map.class); ClientGraphQlResponse response = creatResponse(Collections.singletonMap("data", dataMap)); return response.field(path); } - private ResponseField getField(String path, GraphQLError... errors) { + private GraphQlResponseField getField(String path, GraphQLError... errors) { List list = Arrays.stream(errors).map(GraphQLError::toSpecification).collect(Collectors.toList()); ClientGraphQlResponse response = creatResponse(Collections.singletonMap("errors", list)); return response.field(path); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/client/GraphQlClientTests.java b/spring-graphql/src/test/java/org/springframework/graphql/client/GraphQlClientTests.java index 47fc7e22..49324699 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/client/GraphQlClientTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/client/GraphQlClientTests.java @@ -188,7 +188,7 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { .as("Partial response with field errors should be considered valid") .isTrue(); - ResponseField field = response.field("me"); + GraphQlResponseField field = response.field("me"); assertThat(field.hasValue()).isTrue(); assertThat(field.getErrors()).hasSize(1); assertThat(field.getErrors().get(0).getParsedPath()).containsExactly("me", "name"); @@ -196,7 +196,7 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { .as("Decoding with nested field error should not be precluded") .isNotNull(); - ResponseField nameField = response.field("me.name"); + GraphQlResponseField nameField = response.field("me.name"); assertThat(nameField.hasValue()).isFalse(); assertThat(nameField.getError()).isNotNull(); assertThat(nameField.getError().getParsedPath()).containsExactly("me", "name"); @@ -204,7 +204,7 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { .as("Decoding field null with direct field error should be rejected") .isInstanceOf(FieldAccessException.class); - ResponseField nonExistingField = response.field("me.name.other"); + GraphQlResponseField nonExistingField = response.field("me.name.other"); assertThat(nonExistingField.hasValue()).isFalse(); assertThat(nameField.getError()).isNotNull(); assertThat(nameField.getError().getParsedPath()).containsExactly("me", "name");