From 9102bdbc770cfedba652d668c09799135fa9bd2d Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 17 Mar 2022 13:37:07 +0000 Subject: [PATCH] Replace isValid with hasValue in ResponseField hasValue is more meaningful because it means valid and with some value that could be decoded, which in turn allows removing Nullable from toEntity/List. Or if there is no value, getError() can be used to differentiate between a failed field vs a null field declared optional. See gh-10 --- .../graphql/client/ClientGraphQlResponse.java | 11 +++--- .../client/DefaultClientGraphQlResponse.java | 28 +++++---------- .../graphql/client/DefaultGraphQlClient.java | 10 +++--- .../graphql/client/FieldAccessException.java | 6 ++-- .../graphql/client/GraphQlClient.java | 35 +++++++------------ .../graphql/client/ResponseField.java | 28 +++++++-------- .../graphql/client/GraphQlClientTests.java | 8 ++--- 7 files changed, 51 insertions(+), 75 deletions(-) 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 1580c4f5..afc9bb7d 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 @@ -20,7 +20,6 @@ package org.springframework.graphql.client; import org.springframework.core.ParameterizedTypeReference; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; -import org.springframework.lang.Nullable; /** * {@link GraphQlResponse} for client use, with further options to handle the @@ -49,27 +48,25 @@ 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#isValid()} to check if the - * field actually exists, has a value, or field errors + * decode its value; use {@link ResponseField#hasValue()} to check if the + * field actually exists and has a value. */ ResponseField field(String path); /** * Decode the full response map to the given target type. * @param type the target class - * @return the decoded value, or {@code null} if the "data" is {@code null} + * @return the decoded value, or never {@code null} * @throws FieldAccessException if the response is not {@link #isValid() valid} */ - @Nullable D toEntity(Class type); /** * Variant of {@link #toEntity(Class)} with a {@link ParameterizedTypeReference}. * @param type the target type - * @return the decoded value, or {@code null} if the "data" is {@code null} + * @return the decoded value, or never {@code null} * @throws FieldAccessException if the response is not {@link #isValid() valid} */ - @Nullable D toEntity(ParameterizedTypeReference 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 3fce9c6f..2acea8c4 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 @@ -75,8 +75,7 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C Object value = getFieldValue(dataPath); List errors = getFieldErrors(dataPath); - return new DefaultField( - path, dataPath, (value != NO_VALUE), (value != NO_VALUE ? value : null), errors); + return new DefaultField(path, dataPath, (value != NO_VALUE ? value : null), errors); } @Override @@ -101,18 +100,14 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C private final List errors; - private final boolean exists; - @Nullable private final Object value; public DefaultField( - String path, List parsedPath, boolean exists, @Nullable Object value, - List errors) { + String path, List parsedPath, @Nullable Object value, List errors) { this.path = path; this.parsedPath = parsedPath; - this.exists = exists; this.value = value; this.errors = errors; } @@ -123,8 +118,8 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C } @Override - public boolean isValid() { - return (this.exists && (this.value != null || this.errors.isEmpty())); + public boolean hasValue() { + return (this.value != null); } @SuppressWarnings("unchecked") @@ -160,25 +155,18 @@ final class DefaultClientGraphQlResponse extends MapGraphQlResponse implements C @Override public List toEntityList(Class elementType) { - List list = toEntity(ResolvableType.forClassWithGenerics(List.class, elementType)); - return (list != null ? list : Collections.emptyList()); + return toEntity(ResolvableType.forClassWithGenerics(List.class, elementType)); } @Override public List toEntityList(ParameterizedTypeReference elementType) { - List list = toEntity(ResolvableType.forClassWithGenerics(List.class, ResolvableType.forType(elementType))); - return (list != null ? list : Collections.emptyList()); + return toEntity(ResolvableType.forClassWithGenerics(List.class, ResolvableType.forType(elementType))); } - @SuppressWarnings("unchecked") - @Nullable + @SuppressWarnings({"unchecked", "ConstantConditions"}) private T toEntity(ResolvableType targetType) { - if (!isValid()) { - throw new FieldAccessException(request, DefaultClientGraphQlResponse.this, this); - } - if (this.value == null) { - return null; + throw new FieldAccessException(request, DefaultClientGraphQlResponse.this, this); } DataBufferFactory bufferFactory = DefaultDataBufferFactory.sharedInstance; 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 658a6967..a672cfc5 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,7 +199,7 @@ final class DefaultGraphQlClient implements GraphQlClient { protected ResponseField getField(ClientGraphQlResponse response) { ResponseField field = response.field(this.path); - if (!field.isValid() || !field.getErrors().isEmpty()) { + if (!field.hasValue() || !field.getErrors().isEmpty()) { GraphQlRequest request = response.getRequest(); throw new FieldAccessException(request, response, field); } @@ -220,12 +220,12 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Mono toEntity(Class entityType) { - return this.responseMono.map(this::getField).mapNotNull(field -> field.toEntity(entityType)); + return this.responseMono.map(this::getField).map(field -> field.toEntity(entityType)); } @Override public Mono toEntity(ParameterizedTypeReference entityType) { - return this.responseMono.map(this::getField).mapNotNull(field -> field.toEntity(entityType)); + return this.responseMono.map(this::getField).map(field -> field.toEntity(entityType)); } @Override @@ -252,12 +252,12 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Flux toEntity(Class entityType) { - return this.responseFlux.map(this::getField).mapNotNull(field -> field.toEntity(entityType)); + return this.responseFlux.map(this::getField).map(field -> field.toEntity(entityType)); } @Override public Flux toEntity(ParameterizedTypeReference entityType) { - return this.responseFlux.map(this::getField).mapNotNull(field -> field.toEntity(entityType)); + return this.responseFlux.map(this::getField).map(field -> field.toEntity(entityType)); } @Override 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 69496ea7..6e9d13f1 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 @@ -21,9 +21,9 @@ import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.GraphQlResponse; /** - * An exception raised on an attempt to decode data from an - * {@link GraphQlResponse#isValid() invalid response} or an - * {@link ResponseField#isValid() invalid field}. + * 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()}. * * @author Rossen Stoyanchev * @since 1.0.0 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 35def12a..5238b22e 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 @@ -180,11 +180,9 @@ public interface GraphQlClient { /** * Decode the field to an entity of the given type. * @param entityType the type to convert to - * @return {@code Mono} with the decoded entity, possibly empty if the field - * {@link ResponseField#getValue() value} is {@code null} - * @throws FieldAccessException if the target field is not - * {@link ResponseField#isValid() valid} or has any errors, including - * nested errors. + * @return {@code Mono} with the decoded entity, or a + * {@link FieldAccessException} if the target field is not present or + * has no value, checked via {@link ResponseField#hasValue()}. */ Mono toEntity(Class entityType); @@ -196,12 +194,9 @@ 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 {@code Mono} with the list of decoded entities, possibly an - * empty list if the field {@link ResponseField#getValue() value} is - * {@code null} or empty - * @throws FieldAccessException if the target field is not - * {@link ResponseField#isValid() valid} or has any errors, including - * nested errors. + * @return {@code Mono} with a list of decoded entities, possibly empty, or + * a {@link FieldAccessException} if the target field is not present or + * has no value, checked via {@link ResponseField#hasValue()}; the stream */ Mono> toEntityList(Class elementType); @@ -221,11 +216,9 @@ public interface GraphQlClient { /** * Decode the field to an entity of the given type. * @param entityType the type to convert to - * @return {@code Mono} with the decoded entity, possibly empty if the field - * {@link ResponseField#getValue() value} is {@code null} - * @throws FieldAccessException if the target field is not - * {@link ResponseField#isValid() valid} or has any errors, including - * nested errors. + * @return {@code Mono} with the decoded entity, or a + * {@link FieldAccessException} if the target field is not present or + * has no value, checked via {@link ResponseField#hasValue()}. */ Flux toEntity(Class entityType); @@ -237,12 +230,10 @@ 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 {@code Mono} with the list of decoded entities, possibly an - * empty list if the field {@link ResponseField#getValue() value} is - * {@code null} or empty - * @throws FieldAccessException if the target field is not - * {@link ResponseField#isValid() valid} or has any errors, including - * nested errors. + * @return lists of decoded entities, possibly empty, or a + * {@link FieldAccessException} if the target field is not present or + * has no value, checked via {@link ResponseField#hasValue()}; the stream + * may also end with a range of {@link GraphQlTransportException} types. */ 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/ResponseField.java index 6e063429..e053d20b 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 @@ -34,17 +34,18 @@ import org.springframework.lang.Nullable; public interface ResponseField { /** - * Whether the field is valid. A field is invalid if: + * Whether the field is valid and has a value. *
    - *
  • the path doesn't exist - *
  • it is {@code null} AND has a field error + *
  • {@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. *
- *

A field that is not {@code null} is valid but may still be partial - * with some fields below it set to {@code null} due to field errors. - * A valid field may be {@code null} if the schema allows it, but in that - * case it will not have any field errors. */ - boolean isValid(); + boolean hasValue(); /** * Return the path for the field under the "data" key in the response map. @@ -84,24 +85,23 @@ public interface ResponseField { /** * Decode the field to an entity of the given type. * @param entityType the type to convert to - * @return the decoded entity, possibly {@code null} if the field - * {@link #getValue() value} is {@code null} - * @throws FieldAccessException if "this" field is not {@link #isValid() valid} + * @return the decoded entity, never {@code null} + * @throws FieldAccessException if the target field is not present or + * has no value, checked via {@link #hasValue()}. */ - @Nullable D toEntity(Class entityType); /** * Variant of {@link #toEntity(Class)} with a {@link ParameterizedTypeReference}. */ - @Nullable D toEntity(ParameterizedTypeReference entityType); /** * Decode the field to a list of entities with the given type. * @param elementType the type of elements in the list * @return the decoded list of entities, possibly empty - * @throws FieldAccessException if "this" field is not {@link #isValid() valid} + * @throws FieldAccessException if the target field is not present or + * has no value, checked via {@link #hasValue()}. */ List toEntityList(Class elementType); 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 2148a1de..04dc65a0 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 @@ -165,7 +165,7 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { assertThat(response).isNotNull(); assertThat(response.isValid()).isFalse(); - assertThat(response.field("me").isValid()).isFalse(); + assertThat(response.field("me").hasValue()).isFalse(); assertThatThrownBy(() -> response.field("me").toEntity(MovieCharacter.class)) .isInstanceOf(FieldAccessException.class); @@ -187,7 +187,7 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { .isTrue(); ResponseField field = response.field("me"); - assertThat(field.isValid()).isTrue(); + assertThat(field.hasValue()).isTrue(); assertThat(field.getErrors()).hasSize(1); assertThat(field.getErrors().get(0).getPath()).containsExactly("me", "name"); assertThat(field.toEntity(MovieCharacter.class)) @@ -195,7 +195,7 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { .isNotNull(); ResponseField nameField = response.field("me.name"); - assertThat(nameField.isValid()).isFalse(); + assertThat(nameField.hasValue()).isFalse(); assertThat(nameField.getError()).isNotNull(); assertThat(nameField.getError().getPath()).containsExactly("me", "name"); assertThatThrownBy(() -> nameField.toEntity(String.class)) @@ -203,7 +203,7 @@ public class GraphQlClientTests extends GraphQlClientTestSupport { .isInstanceOf(FieldAccessException.class); ResponseField nonExistingField = response.field("me.name.other"); - assertThat(nonExistingField.isValid()).isFalse(); + assertThat(nonExistingField.hasValue()).isFalse(); assertThat(nameField.getError()).isNotNull(); assertThat(nameField.getError().getPath()).containsExactly("me", "name"); }