From ca90cedf3b0b410cf39fee318174de9ff0de7fb7 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 4 Nov 2022 13:53:52 +0000 Subject: [PATCH] Refine null handling in ClientResponseField#toEntity closes gh-525 --- .../graphql/client/ClientResponseField.java | 28 +++++++++++++++---- .../client/DefaultClientGraphQlResponse.java | 20 +++++++++++-- .../client/DefaultClientResponseField.java | 13 +++++++-- .../graphql/client/DefaultGraphQlClient.java | 13 ++++----- 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/ClientResponseField.java b/spring-graphql/src/main/java/org/springframework/graphql/client/ClientResponseField.java index 0ddbaf25..8d7ef9a9 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/ClientResponseField.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/ClientResponseField.java @@ -20,7 +20,9 @@ package org.springframework.graphql.client; import java.util.List; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.graphql.GraphQlResponse; import org.springframework.graphql.ResponseField; +import org.springframework.lang.Nullable; /** * Extends {@link ResponseField} to add options for decoding the field value. @@ -33,27 +35,41 @@ public interface ClientResponseField extends ResponseField { /** * Decode the field to an entity of the given type. * @param entityType the type to convert to - * @return the decoded entity, never {@code null} - * @throws FieldAccessException if the target field is {@code null} and has + * @return the decoded entity, or {@code null} if the field is {@code null} + * but otherwise there are no errors + * @throws FieldAccessException if the target field is {@code null} and the + * response is not {@link GraphQlResponse#isValid() valid} or the field has * {@link ResponseField#getErrors() errors}. */ + @Nullable D toEntity(Class entityType); /** * Variant of {@link #toEntity(Class)} with a {@link ParameterizedTypeReference}. */ + @Nullable D toEntity(ParameterizedTypeReference entityType); /** * Variant of {@link #toEntity(Class)} to decode to a list of entities. * @param elementType the type of elements in the list + * @return the list of decoded entities, or an empty list if the field is + * {@code null} but otherwise there are no errors + * @throws FieldAccessException if the target field is {@code null} and the + * response is not {@link GraphQlResponse#isValid() valid} or the field has + * {@link ResponseField#getErrors() errors}. */ List toEntityList(Class elementType); - /** - * Variant of {@link #toEntity(Class)} to decode to a list of entities. - * @param elementType the type of elements in the list - */ + /** + * Variant of {@link #toEntity(Class)} to decode to a list of entities. + * @param elementType the type of elements in the list + * @return the list of decoded entities, or an empty list if the field is + * {@code null} but otherwise there are no errors + * @throws FieldAccessException if the target field is {@code null} and the + * response is not {@link GraphQlResponse#isValid() valid} or the field has + * {@link ResponseField#getErrors() errors}. + */ List toEntityList(ParameterizedTypeReference elementType); } 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 5450f971..18a5ad4c 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 @@ -68,12 +68,28 @@ final class DefaultClientGraphQlResponse extends ResponseMapGraphQlResponse impl @Override public D toEntity(Class type) { - return field("").toEntity(type); + ClientResponseField field = field(""); + D entity = field.toEntity(type); + + // should never happen because toEntity checks response.isValid + if (entity == null) { + throw new FieldAccessException(getRequest(), this, field); + } + + return entity; } @Override public D toEntity(ParameterizedTypeReference type) { - return field("").toEntity(type); + ClientResponseField field = field(""); + D entity = field.toEntity(type); + + // should never happen because toEntity checks response.isValid + if (entity == null) { + throw new FieldAccessException(getRequest(), this, field); + } + + return entity; } } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientResponseField.java b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientResponseField.java index 41e4c004..44646250 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientResponseField.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultClientResponseField.java @@ -29,6 +29,7 @@ import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.graphql.ResponseError; import org.springframework.graphql.ResponseField; +import org.springframework.lang.Nullable; import org.springframework.util.MimeType; import org.springframework.util.MimeTypeUtils; @@ -98,17 +99,23 @@ final class DefaultClientResponseField implements ClientResponseField { @Override public List toEntityList(Class elementType) { - return toEntity(ResolvableType.forClassWithGenerics(List.class, elementType)); + List list = toEntity(ResolvableType.forClassWithGenerics(List.class, elementType)); + return (list != null ? list : Collections.emptyList()); } @Override public List toEntityList(ParameterizedTypeReference elementType) { - return toEntity(ResolvableType.forClassWithGenerics(List.class, ResolvableType.forType(elementType))); + List list = toEntity(ResolvableType.forClassWithGenerics(List.class, ResolvableType.forType(elementType))); + return (list != null ? list : Collections.emptyList()); } - @SuppressWarnings({"unchecked", "ConstantConditions"}) + @SuppressWarnings("unchecked") + @Nullable private T toEntity(ResolvableType targetType) { if (getValue() == null) { + if (this.response.isValid() && getErrors().isEmpty()) { + return null; + } throw new FieldAccessException(this.response.getRequest(), this.response, this); } 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 a2f5975f..a43e4a44 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,9 +186,8 @@ final class DefaultGraphQlClient implements GraphQlClient { } /** - * Return the field or {@code null}, but only if the response is valid - * and there are no field errors, or raise {@link FieldAccessException} - * otherwise. + * Return the field or {@code null}, but only if the response is valid and + * there are no field errors. Raise {@link FieldAccessException} otherwise. * @throws FieldAccessException in case of an invalid response or any * field error at, above or below the field path */ @@ -216,12 +215,12 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Mono toEntity(Class entityType) { - return this.responseMono.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType)); + return this.responseMono.mapNotNull(this::getValidField).mapNotNull(field -> field.toEntity(entityType)); } @Override public Mono toEntity(ParameterizedTypeReference entityType) { - return this.responseMono.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType)); + return this.responseMono.mapNotNull(this::getValidField).mapNotNull(field -> field.toEntity(entityType)); } @Override @@ -254,12 +253,12 @@ final class DefaultGraphQlClient implements GraphQlClient { @Override public Flux toEntity(Class entityType) { - return this.responseFlux.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType)); + return this.responseFlux.mapNotNull(this::getValidField).mapNotNull(field -> field.toEntity(entityType)); } @Override public Flux toEntity(ParameterizedTypeReference entityType) { - return this.responseFlux.mapNotNull(this::getValidField).map(field -> field.toEntity(entityType)); + return this.responseFlux.mapNotNull(this::getValidField).mapNotNull(field -> field.toEntity(entityType)); } @Override