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

Author: rstoyanchev

spring-graphql/src/main/java/org/springframework/graphql/client/ClientGraphQlResponse.java

@@ -20,7 +20,6 @@
 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 {
 	 * </pre>
 	 * @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> D toEntity(Class<D> 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> D toEntity(ParameterizedTypeReference<D> type);
 
 }

spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientGraphQlResponse.java

@@ -75,8 +75,7 @@ public ResponseField field(String path) {
 		Object value = getFieldValue(dataPath);
 		List<GraphQLError> 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 @@ private class DefaultField implements ResponseField {
 
 		private final List<GraphQLError> errors;
 
-		private final boolean exists;
-
 		@Nullable
 		private final Object value;
 
 		public DefaultField(
-				String path, List<Object> parsedPath, boolean exists, @Nullable Object value,
-				List<GraphQLError> errors) {
+				String path, List<Object> parsedPath, @Nullable Object value, List<GraphQLError> errors) {
 
 			this.path = path;
 			this.parsedPath = parsedPath;
-			this.exists = exists;
 			this.value = value;
 			this.errors = errors;
 		}
@@ -123,8 +118,8 @@ public String getPath() {
 		}
 
 		@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 @@ public <D> D toEntity(ParameterizedTypeReference<D> entityType) {
 
 		@Override
 		public <D> List<D> toEntityList(Class<D> elementType) {
-			List<D> list = toEntity(ResolvableType.forClassWithGenerics(List.class, elementType));
-			return (list != null ? list : Collections.emptyList());
+			return toEntity(ResolvableType.forClassWithGenerics(List.class, elementType));
 		}
 
 		@Override
 		public <D> List<D> toEntityList(ParameterizedTypeReference<D> elementType) {
-			List<D> 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> 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;

spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java

@@ -199,7 +199,7 @@ protected RetrieveSpecSupport(String path) {
 
 		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 @@ private static class DefaultRetrieveSpec extends RetrieveSpecSupport implements
 
 		@Override
 		public <D> Mono<D> toEntity(Class<D> 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 <D> Mono<D> toEntity(ParameterizedTypeReference<D> 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 @@ private static class DefaultRetrieveSubscriptionSpec extends RetrieveSpecSupport
 
 		@Override
 		public <D> Flux<D> toEntity(Class<D> 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 <D> Flux<D> toEntity(ParameterizedTypeReference<D> entityType) {
-			return this.responseFlux.map(this::getField).mapNotNull(field -> field.toEntity(entityType));
+			return this.responseFlux.map(this::getField).map(field -> field.toEntity(entityType));
 		}
 
 		@Override

spring-graphql/src/main/java/org/springframework/graphql/client/FieldAccessException.java

@@ -21,9 +21,9 @@
 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

spring-graphql/src/main/java/org/springframework/graphql/client/GraphQlClient.java

@@ -180,11 +180,9 @@ interface RetrieveSpec {
 		/**
 		 * 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()}.
 		 */
 		<D> Mono<D> toEntity(Class<D> entityType);
 
@@ -196,12 +194,9 @@ interface RetrieveSpec {
 		/**
 		 * 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
 		 */
 		<D> Mono<List<D>> toEntityList(Class<D> elementType);
 
@@ -221,11 +216,9 @@ interface RetrieveSubscriptionSpec {
 		/**
 		 * 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()}.
 		 */
 		<D> Flux<D> toEntity(Class<D> entityType);
 
@@ -237,12 +230,10 @@ interface RetrieveSubscriptionSpec {
 		/**
 		 * 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.
 		 */
 		<D> Flux<List<D>> toEntityList(Class<D> elementType);
 

spring-graphql/src/main/java/org/springframework/graphql/client/ResponseField.java

@@ -34,17 +34,18 @@
 public interface ResponseField {
 
 	/**
-	 * Whether the field is valid. A field is invalid if:
+	 * Whether the field is valid and has a value.
 	 * <ul>
-	 * <li>the path doesn't exist
-	 * <li>it is {@code null} AND has a field error
+	 * <li>{@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}.
+	 * <li>{@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.
 	 * </ul>
-	 * <p>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> D toEntity(Class<D> entityType);
 
 	/**
 	 * Variant of {@link #toEntity(Class)} with a {@link ParameterizedTypeReference}.
 	 */
-	@Nullable
 	<D> D toEntity(ParameterizedTypeReference<D> 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()}.
 	 */
 	<D> List<D> toEntityList(Class<D> elementType);
 

spring-graphql/src/test/java/org/springframework/graphql/client/GraphQlClientTests.java

@@ -165,7 +165,7 @@ private void testExecuteFailedResponse(String document) {
 
 		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,23 +187,23 @@ void executePartialResponse() {
 				.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))
 				.as("Decoding with nested field error should not be precluded")
 				.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))
 				.as("Decoding field null with direct field error should be rejected")
 				.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");
 	}