Refine null handling in GraphQlClient for retrieve

If a field is null but without errors, i.e. declared optional in the
schema, it is more natural for toEntity to complete empty instead of
raising a FieldAccessException.

This aligns better with the execute method where handling the field
directly allows treating a null but valid field as optional.

See gh-10

Author: rstoyanchev

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

@@ -15,6 +15,7 @@
  */
 package org.springframework.graphql.client;
 
+import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -197,13 +198,17 @@ protected RetrieveSpecSupport(String path) {
 			this.path = path;
 		}
 
-		protected ResponseField getField(ClientGraphQlResponse response) {
+		/**
+		 * Return the field if valid, possibly {@code null}.
+		 * @throws FieldAccessException if the response or field is not valid
+		 */
+		@Nullable
+		protected ResponseField getValidField(ClientGraphQlResponse response) {
 			ResponseField field = response.field(this.path);
-			if (!field.hasValue() || !field.getErrors().isEmpty()) {
-				GraphQlRequest request = response.getRequest();
-				throw new FieldAccessException(request, response, field);
+			if (!response.isValid() || field.getError() != null) {
+				throw new FieldAccessException(response.getRequest(), response, field);
 			}
-			return field;
+			return (field.hasValue() ? field : null);
 		}
 
 	}
@@ -220,22 +225,28 @@ private static class DefaultRetrieveSpec extends RetrieveSpecSupport implements
 
 		@Override
 		public <D> Mono<D> toEntity(Class<D> entityType) {
-			return this.responseMono.map(this::getField).map(field -> field.toEntity(entityType));
+			return this.responseMono.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType));
 		}
 
 		@Override
 		public <D> Mono<D> toEntity(ParameterizedTypeReference<D> entityType) {
-			return this.responseMono.map(this::getField).map(field -> field.toEntity(entityType));
+			return this.responseMono.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType));
 		}
 
 		@Override
 		public <D> Mono<List<D>> toEntityList(Class<D> elementType) {
-			return this.responseMono.map(this::getField).map(field -> field.toEntityList(elementType));
+			return this.responseMono.map(response -> {
+				ResponseField field = getValidField(response);
+				return (field != null ? field.toEntityList(elementType) : Collections.emptyList());
+			});
 		}
 
 		@Override
 		public <D> Mono<List<D>> toEntityList(ParameterizedTypeReference<D> elementType) {
-			return this.responseMono.map(this::getField).map(field -> field.toEntityList(elementType));
+			return this.responseMono.map(response -> {
+				ResponseField field = getValidField(response);
+				return (field != null ? field.toEntityList(elementType) : Collections.emptyList());
+			});
 		}
 
 	}
@@ -252,22 +263,28 @@ private static class DefaultRetrieveSubscriptionSpec extends RetrieveSpecSupport
 
 		@Override
 		public <D> Flux<D> toEntity(Class<D> entityType) {
-			return this.responseFlux.map(this::getField).map(field -> field.toEntity(entityType));
+			return this.responseFlux.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType));
 		}
 
 		@Override
 		public <D> Flux<D> toEntity(ParameterizedTypeReference<D> entityType) {
-			return this.responseFlux.map(this::getField).map(field -> field.toEntity(entityType));
+			return this.responseFlux.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType));
 		}
 
 		@Override
 		public <D> Flux<List<D>> toEntityList(Class<D> elementType) {
-			return this.responseFlux.map(this::getField).map(field -> field.toEntityList(elementType));
+			return this.responseFlux.map(response -> {
+				ResponseField field = getValidField(response);
+				return (field != null ? field.toEntityList(elementType) : Collections.emptyList());
+			});
 		}
 
 		@Override
 		public <D> Flux<List<D>> toEntityList(ParameterizedTypeReference<D> elementType) {
-			return this.responseFlux.map(this::getField).map(field -> field.toEntityList(elementType));
+			return this.responseFlux.map(response -> {
+				ResponseField field = getValidField(response);
+				return (field != null ? field.toEntityList(elementType) : Collections.emptyList());
+			});
 		}
 
 	}

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

@@ -155,8 +155,8 @@ interface RequestSpec {
 
 		/**
 		 * Execute a "subscription" request and return a stream of responses.
-		 * @return a {@code Flux} with a {@code ClientGraphQlResponse} for further
-		 * decoding of the response. The {@code Flux} may terminate as follows:
+		 * @return a {@code Flux} with responses that provide further options for
+		 * decoding of each response. The {@code Flux} may terminate as follows:
 		 * <ul>
 		 * <li>Completes if the subscription completes before the connection is closed.
 		 * <li>{@link SubscriptionErrorException} if the subscription ends with an error.
@@ -180,9 +180,10 @@ 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, or a
-		 * {@link FieldAccessException} if the target field is not present or
-		 * has no value, checked via {@link ResponseField#hasValue()}.
+		 * @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.
 		 */
 		<D> Mono<D> toEntity(Class<D> entityType);
 
@@ -194,9 +195,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 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
+		 * @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.
 		 */
 		<D> Mono<List<D>> toEntityList(Class<D> elementType);
 
@@ -216,9 +217,11 @@ 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, or a
+		 * @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, checked via {@link ResponseField#hasValue()}.
+		 * has no value in a given response; the stream may also end with a
+		 * {@link GraphQlTransportException}.
 		 */
 		<D> Flux<D> toEntity(Class<D> entityType);
 
@@ -230,10 +233,11 @@ interface RetrieveSubscriptionSpec {
 		/**
 		 * 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, possibly empty, or a
+		 * @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, checked via {@link ResponseField#hasValue()}; the stream
-		 * may also end with a range of {@link GraphQlTransportException} types.
+		 * has no value in a given response; the stream may also end with a
+		 * {@link GraphQlTransportException}.
 		 */
 		<D> Flux<List<D>> toEntityList(Class<D> elementType);
 

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

@@ -34,7 +34,7 @@
 public interface ResponseField {
 
 	/**
-	 * Whether the field is valid and has a value.
+	 * Whether the field has a value.
 	 * <ul>
 	 * <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

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

@@ -133,7 +133,9 @@ void retrievePartialResponse() {
 		String document = "fieldErrorResponse";
 		initResponse(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();
+
 		testRetrieveFieldAccessException(document, "me.name");
 	}