From b0dedbeb64d9768ecd950affec086d24699f4fd2 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 22 Sep 2021 19:02:33 +0200 Subject: [PATCH] Polish "Recursively instantiate arguments" Closes gh-147 --- .../ArgumentMethodArgumentResolver.java | 7 +-- .../support/GraphQlArgumentInstantiator.java | 49 +++++++++++++++---- .../GraphQlArgumentInstantiatorTests.java | 12 ++--- 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java index 0145b3a2..7bc5f647 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java @@ -80,11 +80,8 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso if (CollectionFactory.isApproximableCollectionType(rawValue.getClass())) { Assert.isAssignable(Collection.class, parameterType.getType(), "Argument '" + name + "' is a Collection while the @Argument method parameter is " + parameterType.getType()); - Collection rawCollection = (Collection) rawValue; - Collection values = CollectionFactory.createApproximateCollection(rawValue, rawCollection.size()); Class elementType = parameterType.getElementTypeDescriptor().getType(); - rawCollection.forEach(item -> values.add(convert(item, elementType))); - return values; + return this.instantiator.instantiateCollection(elementType, (Collection) rawValue); } MethodParameter nestedParameter = parameter.nestedIfOptional(); @@ -103,7 +100,7 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso private Object convert(Object rawValue, Class targetType) { Object target; if (rawValue instanceof Map) { - target = this.instantiator.instantiate(targetType, (Map) rawValue); + target = this.instantiator.instantiate((Map) rawValue, targetType); } else if (targetType.isAssignableFrom(rawValue.getClass())) { return returnValue(rawValue, targetType); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java index 8ff82027..4ff76915 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java @@ -26,10 +26,10 @@ import java.util.Stack; import org.springframework.beans.BeanUtils; import org.springframework.beans.MutablePropertyValues; -import org.springframework.beans.PropertyValues; import org.springframework.core.CollectionFactory; import org.springframework.core.MethodParameter; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.util.Assert; import org.springframework.validation.DataBinder; /** @@ -40,20 +40,22 @@ import org.springframework.validation.DataBinder; */ class GraphQlArgumentInstantiator { + private final DataBinder converter = new DataBinder(null); + /** * Instantiate the given target type and bind data from * {@link graphql.schema.DataFetchingEnvironment} arguments. *

This is considering the default constructor or a primary constructor * if available. * - * @param targetType the type of the argument to instantiate * @param arguments the data fetching environment arguments + * @param targetType the type of the argument to instantiate * @param the type of the input argument * @return the instantiated and populated input argument. * @throws IllegalStateException if there is no suitable constructor. */ @SuppressWarnings("unchecked") - public T instantiate(Class targetType, Map arguments) { + public T instantiate(Map arguments, Class targetType) { Object target; Constructor ctor = BeanUtils.getResolvableConstructor(targetType); @@ -65,7 +67,6 @@ class GraphQlArgumentInstantiator { } else { // Data class constructor - DataBinder binder = new DataBinder(null); String[] paramNames = BeanUtils.getParameterNames(ctor); Class[] paramTypes = ctor.getParameterTypes(); Object[] args = new Object[paramTypes.length]; @@ -77,16 +78,12 @@ class GraphQlArgumentInstantiator { args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null); } else if (value != null && CollectionFactory.isApproximableCollectionType(value.getClass())) { - Collection rawCollection = (Collection) value; - Collection values = CollectionFactory.createApproximateCollection(value, rawCollection.size()); - TypeDescriptor typeDescriptor = new TypeDescriptor(methodParam); Class elementType = typeDescriptor.getElementTypeDescriptor().getType(); - rawCollection.forEach(item -> values.add(this.instantiate(elementType, (Map)item))); - args[i] = values; + args[i] = instantiateCollection(elementType, (Collection) value); } else { - args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam); + args[i] = this.converter.convertIfNecessary(value, paramTypes[i], methodParam); } } target = BeanUtils.instantiateClass(ctor, args); @@ -94,6 +91,38 @@ class GraphQlArgumentInstantiator { return (T) target; } + /** + * Instantiate a collection of {@code elementType} using the given {@code values}. + *

This will instantiate a new Collection of the closest type possible + * from the one provided as an argument. + * + * @param elementType the type of elements in the given Collection + * @param values the collection of values to bind and instantiate + * @param the type of Collection elements + * @return the instantiated and populated Collection. + * @throws IllegalStateException if there is no suitable constructor. + */ + @SuppressWarnings("unchecked") + public Collection instantiateCollection(Class elementType, Collection values) { + Assert.state(CollectionFactory.isApproximableCollectionType(values.getClass()), + () -> "Cannot instantiate Collection for type " + values.getClass()); + Collection instances = CollectionFactory.createApproximateCollection(values, values.size()); + values.forEach(item -> { + T value; + if (elementType.isAssignableFrom(item.getClass())) { + value = (T) item; + } + else if (item instanceof Map) { + value = this.instantiate((Map)item, elementType); + } + else { + value = this.converter.convertIfNecessary(item, elementType); + } + instances.add(value); + }); + return instances; + } + /** * Perform a Depth First Search in the given JSON map to collect attribute values * as {@link MutablePropertyValues} using the full property path as key. diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java index 2519f686..ee5a9092 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java @@ -46,7 +46,7 @@ class GraphQlArgumentInstantiatorTests { void shouldInstantiateDefaultConstructor() throws Exception { String payload = "{\"simpleBean\": { \"name\": \"test\"} }"; DataFetchingEnvironment environment = initEnvironment(payload); - SimpleBean result = instantiator.instantiate(SimpleBean.class, environment.getArgument("simpleBean")); + SimpleBean result = instantiator.instantiate(environment.getArgument("simpleBean"), SimpleBean.class); assertThat(result).isNotNull().isInstanceOf(SimpleBean.class); assertThat(result).hasFieldOrPropertyWithValue("name", "test"); @@ -56,7 +56,7 @@ class GraphQlArgumentInstantiatorTests { void shouldInstantiatePrimaryConstructor() throws Exception { String payload = "{\"constructorBean\": { \"name\": \"test\"} }"; DataFetchingEnvironment environment = initEnvironment(payload); - ContructorBean result = instantiator.instantiate(ContructorBean.class, environment.getArgument("constructorBean")); + ContructorBean result = instantiator.instantiate(environment.getArgument("constructorBean"), ContructorBean.class); assertThat(result).isNotNull().isInstanceOf(ContructorBean.class); assertThat(result).hasFieldOrPropertyWithValue("name", "test"); @@ -66,7 +66,7 @@ class GraphQlArgumentInstantiatorTests { void shouldFailIfNoPrimaryConstructor() throws Exception { String payload = "{\"noPrimary\": { \"name\": \"test\"} }"; DataFetchingEnvironment environment = initEnvironment(payload); - assertThatThrownBy(() -> instantiator.instantiate(NoPrimaryConstructor.class, environment.getArgument("noPrimary"))) + assertThatThrownBy(() -> instantiator.instantiate(environment.getArgument("noPrimary"), NoPrimaryConstructor.class)) .isInstanceOf(IllegalStateException.class).hasMessageContaining("No primary or single public constructor found"); } @@ -74,7 +74,7 @@ class GraphQlArgumentInstantiatorTests { void shouldInstantiateNestedBean() throws Exception { String payload = "{\"book\": { \"name\": \"test name\", \"author\": { \"firstName\": \"Jane\", \"lastName\": \"Spring\"} } }"; DataFetchingEnvironment environment = initEnvironment(payload); - Book result = instantiator.instantiate(Book.class, environment.getArgument("book")); + Book result = instantiator.instantiate(environment.getArgument("book"), Book.class); assertThat(result).isNotNull().isInstanceOf(Book.class); assertThat(result).hasFieldOrPropertyWithValue("name", "test name"); @@ -87,7 +87,7 @@ class GraphQlArgumentInstantiatorTests { void shouldInstantiateNestedBeanLists() throws Exception { String payload = "{\"nestedList\": { \"items\": [ {\"name\": \"first\"}, {\"name\": \"second\"}] } }"; DataFetchingEnvironment environment = initEnvironment(payload); - NestedList result = instantiator.instantiate(NestedList.class, environment.getArgument("nestedList")); + NestedList result = instantiator.instantiate(environment.getArgument("nestedList"), NestedList.class); assertThat(result).isNotNull().isInstanceOf(NestedList.class); assertThat(result.getItems()).hasSize(2).extracting("name").containsExactly("first", "second"); @@ -97,7 +97,7 @@ class GraphQlArgumentInstantiatorTests { void shouldInstantiatePrimaryConstructorNestedBeanLists() throws Exception { String payload = "{\"nestedList\": { \"items\": [ {\"name\": \"first\"}, {\"name\": \"second\"}] } }"; DataFetchingEnvironment environment = initEnvironment(payload); - PrimaryConstructorNestedList result = instantiator.instantiate(PrimaryConstructorNestedList.class, environment.getArgument("nestedList")); + PrimaryConstructorNestedList result = instantiator.instantiate(environment.getArgument("nestedList"), PrimaryConstructorNestedList.class); assertThat(result).isNotNull().isInstanceOf(PrimaryConstructorNestedList.class); assertThat(result.getItems()).hasSize(2).extracting("name").containsExactly("first", "second");