From 02f96b6bf415d200d988e6debfc0561397836367 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 3 Nov 2022 17:11:23 +0000 Subject: [PATCH] Raise exception for any field error in retrieve This commit aligns the behavior of retrieve with its Javadoc such that any field error, including errors on nested fields, raises a FieldAccessException Closes gh-499 --- .../graphql/client/DefaultGraphQlClient.java | 9 ++++++--- .../graphql/client/GraphQlClient.java | 4 ++-- .../support/AbstractGraphQlResponse.java | 4 ++-- .../graphql/client/GraphQlClientTests.java | 17 +++++++++++++---- 4 files changed, 23 insertions(+), 11 deletions(-) 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 929578c5..e63adf9b 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 @@ -186,13 +186,16 @@ final class DefaultGraphQlClient implements GraphQlClient { } /** - * Return the field if valid, or {@code null} if {@code null} without errors. - * @throws FieldAccessException for invalid response or failed field + * Return the field or {@code null}, but only if the response is valid + * and there are no field errors, or raise {@link FieldAccessException} + * otherwise. + * @throws FieldAccessException in case of an invalid response or any + * field error at, above or below the field path */ @Nullable protected ClientResponseField getValidField(ClientGraphQlResponse response) { ClientResponseField field = response.field(this.path); - if (!response.isValid() || field.getError() != null) { + if (!response.isValid() || !field.getErrors().isEmpty()) { throw new FieldAccessException( ((DefaultClientGraphQlResponse) response).getRequest(), response, 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 e73a5d42..fb3c6a8d 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 @@ -191,8 +191,8 @@ public interface GraphQlClient { * client.document("..").execute().map(response -> response.toEntity(..)) * * @return a spec with decoding options - * @throws FieldAccessException if the target field has any errors, - * including nested errors. + * @throws FieldAccessException if the field has any field errors, + * including errors at, above or below the field path. */ RetrieveSpec retrieve(String path); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/support/AbstractGraphQlResponse.java b/spring-graphql/src/main/java/org/springframework/graphql/support/AbstractGraphQlResponse.java index 965779a6..85e1a9b2 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/support/AbstractGraphQlResponse.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/support/AbstractGraphQlResponse.java @@ -133,7 +133,7 @@ public abstract class AbstractGraphQlResponse implements GraphQlResponse { } /** - * Return field errors whose path starts with the given field path. + * Return errors whose path is at, above, or below the given path. * @param path the field path to match * @return errors whose path starts with the dataPath */ @@ -144,7 +144,7 @@ public abstract class AbstractGraphQlResponse implements GraphQlResponse { return response.getErrors().stream() .filter(error -> { String errorPath = error.getPath(); - return !errorPath.isEmpty() && (errorPath.startsWith(path) || path.startsWith(errorPath)); + return (!errorPath.isEmpty() && (errorPath.startsWith(path) || path.startsWith(errorPath))); }) .collect(Collectors.toList()); } 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 8fc86da0..b4e4cb2b 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 @@ -130,14 +130,23 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { } @Test - void retrievePartialResponse() { + void retrieveFieldErrorAt() { + String document = "fieldErrorResponse"; + getGraphQlService().setDataAsJsonAndErrors(document, "{\"me\": null}", errorForPath("/me")); + testRetrieveFieldAccessException(document, "me"); + } + @Test // gh-499 + void retrieveFieldErrorBelow() { String document = "fieldErrorResponse"; getGraphQlService().setDataAsJsonAndErrors(document, "{\"me\": {\"name\":null}}", errorForPath("/me/name")); + testRetrieveFieldAccessException(document, "me"); + } - MovieCharacter character = graphQlClient().document(document).retrieve("me").toEntity(MovieCharacter.class).block(); - assertThat(character).isNotNull().extracting(MovieCharacter::getName).isNull(); - + @Test + void retrieveFieldErrorAbove() { + String document = "fieldErrorResponse"; + getGraphQlService().setDataAsJsonAndErrors(document, "{\"me\": null}", errorForPath("/me")); testRetrieveFieldAccessException(document, "me.name"); }