From 90e23945730d6b488578e008f0d9c0fbaf91e3b3 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 5 Jul 2022 07:15:01 +0100 Subject: [PATCH 1/3] Remove ThreadLocal in GraphQlArgumentBinderTests --- .../support/GraphQlArgumentBinderTests.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentBinderTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentBinderTests.java index 00f9cb63..ba942f06 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentBinderTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentBinderTests.java @@ -45,14 +45,13 @@ class GraphQlArgumentBinderTests { private final ObjectMapper mapper = new ObjectMapper(); - private final ThreadLocal initializer = - ThreadLocal.withInitial(() -> new GraphQlArgumentBinder(null)); + private final GraphQlArgumentBinder binder = new GraphQlArgumentBinder(null); @Test void defaultConstructor() throws Exception { - Object result = initializer.get().bind( + Object result = this.binder.bind( environment("{\"key\":{\"name\":\"test\"}}"), "key", ResolvableType.forClass(SimpleBean.class)); @@ -63,7 +62,7 @@ class GraphQlArgumentBinderTests { @Test void defaultConstructorWithNestedBeanProperty() throws Exception { - Object result = initializer.get().bind( + Object result = this.binder.bind( environment( "{\"key\":{" + "\"name\":\"test name\"," + @@ -84,7 +83,7 @@ class GraphQlArgumentBinderTests { @Test void defaultConstructorWithNestedBeanListProperty() throws Exception { - Object result = initializer.get().bind( + Object result = this.binder.bind( environment("{\"key\":{\"items\":[{\"name\":\"first\"},{\"name\":\"second\"}]}}"), "key", ResolvableType.forClass(ItemListHolder.class)); @@ -96,7 +95,7 @@ class GraphQlArgumentBinderTests { @Test // gh-301 void defaultConstructorWithNestedBeanListEmpty() throws Exception { - Object result = initializer.get().bind( + Object result = this.binder.bind( environment("{\"key\":{\"items\": []}}"), "key", ResolvableType.forClass(ItemListHolder.class)); @@ -108,7 +107,7 @@ class GraphQlArgumentBinderTests { void defaultConstructorBindingError() { assertThatThrownBy( - () -> initializer.get().bind( + () -> this.binder.bind( environment("{\"key\":{\"name\":\"test\",\"age\":\"invalid\"}}"), "key", ResolvableType.forClass(SimpleBean.class))) .extracting(ex -> ((BindException) ex).getFieldErrors()) @@ -123,7 +122,7 @@ class GraphQlArgumentBinderTests { @Test void primaryConstructor() throws Exception { - Object result = initializer.get().bind( + Object result = this.binder.bind( environment("{\"key\":{\"name\":\"test\"}}"), "key", ResolvableType.forClass(PrimaryConstructorBean.class)); @@ -134,7 +133,7 @@ class GraphQlArgumentBinderTests { @Test void primaryConstructorWithBeanArgument() throws Exception { - Object result = initializer.get().bind( + Object result = this.binder.bind( environment( "{\"key\":{" + "\"item\":{\"name\":\"Item name\"}," + @@ -152,7 +151,7 @@ class GraphQlArgumentBinderTests { @Test void primaryConstructorWithNestedBeanList() throws Exception { - Object result = initializer.get().bind( + Object result = this.binder.bind( environment( "{\"key\":{\"items\":[" + "{\"name\":\"first\"}," + @@ -168,7 +167,7 @@ class GraphQlArgumentBinderTests { @Test void primaryConstructorNotFound() { assertThatThrownBy( - () -> initializer.get().bind( + () -> this.binder.bind( environment("{\"key\":{\"name\":\"test\"}}"), "key", ResolvableType.forClass(NoPrimaryConstructorBean.class))) .isInstanceOf(IllegalStateException.class) @@ -179,7 +178,7 @@ class GraphQlArgumentBinderTests { void primaryConstructorBindingError() { assertThatThrownBy( - () -> initializer.get().bind( + () -> this.binder.bind( environment( "{\"key\":{" + "\"name\":\"Hello\"," + @@ -205,7 +204,7 @@ class GraphQlArgumentBinderTests { void primaryConstructorBindingErrorWithNestedBeanList() { assertThatThrownBy( - () -> initializer.get().bind( + () -> this.binder.bind( environment( "{\"key\":{\"items\":[" + "{\"name\":\"first\", \"age\":\"invalid\"}," + From e173bef291e2f21c6d3a55bc1752e7eb9eb7b75e Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 5 Jul 2022 07:19:23 +0100 Subject: [PATCH 2/3] Move GraphQlArgumentBinderTests to correct package --- .../annotation/support => }/GraphQlArgumentBinderTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename spring-graphql/src/test/java/org/springframework/graphql/data/{method/annotation/support => }/GraphQlArgumentBinderTests.java (99%) diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentBinderTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java similarity index 99% rename from spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentBinderTests.java rename to spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java index ba942f06..f986a336 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentBinderTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.graphql.data.method.annotation.support; +package org.springframework.graphql.data; import java.util.List; import java.util.Map; From 28e3a8c9a3924cabcf7e6d3c993d9d02626deec9 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 5 Jul 2022 07:32:13 +0100 Subject: [PATCH 3/3] Relax List input check in GraphQlArgumentBinder Allow any List type, not only ArrayList and LinkedList but also others like SingletonList. Fixes gh-410 --- .../graphql/data/GraphQlArgumentBinder.java | 9 +++++-- .../data/GraphQlArgumentBinderTests.java | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java b/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java index 7388491e..11508ec3 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java @@ -128,7 +128,7 @@ public class GraphQlArgumentBinder { try { // From Collection - if (CollectionFactory.isApproximableCollectionType(rawValue.getClass())) { + if (isApproximableCollectionType(rawValue)) { segments.push(argumentName); return createCollection((Collection) rawValue, targetType, bindingResult, segments); } @@ -164,6 +164,11 @@ public class GraphQlArgumentBinder { return (type.resolve(Object.class).equals(Optional.class) ? Optional.ofNullable(value) : value); } + private boolean isApproximableCollectionType(Object rawValue) { + return (CollectionFactory.isApproximableCollectionType(rawValue.getClass()) || + rawValue instanceof List); // it may be SingletonList + } + @SuppressWarnings({"ConstantConditions", "unchecked"}) private Collection createCollection( Collection rawCollection, ResolvableType collectionType, @@ -253,7 +258,7 @@ public class GraphQlArgumentBinder { if (rawValue == null && methodParam.isOptional()) { args[i] = (paramTypes[i] == Optional.class ? Optional.empty() : null); } - else if (rawValue != null && CollectionFactory.isApproximableCollectionType(rawValue.getClass())) { + else if (rawValue != null && isApproximableCollectionType(rawValue)) { ResolvableType elementType = ResolvableType.forMethodParameter(methodParam); args[i] = createCollection((Collection) rawValue, elementType, bindingResult, segments); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java index f986a336..89949929 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java @@ -16,6 +16,8 @@ package org.springframework.graphql.data; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -224,6 +226,31 @@ class GraphQlArgumentBinderTests { }); } + @Test // gh-410 + void coercionWithSingletonList() throws Exception { + + Map itemMap = new HashMap<>(); + itemMap.put("name", "Joe"); + itemMap.put("age", "37"); + + Map arguments = new HashMap<>(); + arguments.put("key", Collections.singletonList(itemMap)); + + DataFetchingEnvironment environment = + DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build(); + + Object result = this.binder.bind(environment, "key", + ResolvableType.forClassWithGenerics(List.class, Item.class)); + + assertThat(result).isNotNull().isInstanceOf(List.class); + List items = (List) result; + + assertThat(items).hasSize(1); + assertThat(items.get(0).getName()).isEqualTo("Joe"); + assertThat(items.get(0).getAge()).isEqualTo(37); + } + + @SuppressWarnings("unchecked") private DataFetchingEnvironment environment(String jsonPayload) throws JsonProcessingException { Map arguments = this.mapper.readValue(jsonPayload, Map.class);