diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedDataFetcherConfigurer.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedDataFetcherConfigurer.java index 79e8894c..acb3757e 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedDataFetcherConfigurer.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedDataFetcherConfigurer.java @@ -159,8 +159,8 @@ public class AnnotatedDataFetcherConfigurer @Override public void afterPropertiesSet() { this.argumentResolvers = new HandlerMethodArgumentResolverComposite(); - this.argumentResolvers.addResolver(initInputArgumentMethodArgumentResolver()); this.argumentResolvers.addResolver(new ArgumentMapMethodArgumentResolver()); + this.argumentResolvers.addResolver(new ArgumentMethodArgumentResolver()); this.argumentResolvers.addResolver(new DataFetchingEnvironmentMethodArgumentResolver()); this.argumentResolvers.addResolver(new DataLoaderMethodArgumentResolver()); @@ -172,22 +172,6 @@ public class AnnotatedDataFetcherConfigurer this.argumentResolvers.addResolver(new SourceMethodArgumentResolver()); } - private ArgumentMethodArgumentResolver initInputArgumentMethodArgumentResolver() { - ArgumentMethodArgumentResolver argumentResolver; - if (this.jsonMessageConverter != null) { - argumentResolver = new ArgumentMethodArgumentResolver(this.jsonMessageConverter); - } - else if (this.jsonEncoder != null && this.jsonDecoder != null) { - argumentResolver = new ArgumentMethodArgumentResolver(this.jsonDecoder, this.jsonEncoder); - } - else { - throw new IllegalArgumentException( - "Neither HttpMessageConverter nor Encoder/Decoder for JSON provided"); - } - return argumentResolver; - } - - @Override public void configure(RuntimeWiring.Builder builder) { Assert.notNull(this.applicationContext, "ApplicationContext is required"); 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 4ed637e8..f8950e9b 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 @@ -15,35 +15,26 @@ */ package org.springframework.graphql.data.method.annotation.support; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.Collections; -import java.util.List; +import java.lang.reflect.Constructor; +import java.util.Collection; +import java.util.Iterator; +import java.util.Map; import java.util.Optional; +import java.util.Stack; import graphql.schema.DataFetchingEnvironment; +import org.springframework.beans.BeanUtils; +import org.springframework.beans.MutablePropertyValues; +import org.springframework.core.CollectionFactory; import org.springframework.core.MethodParameter; -import org.springframework.core.ResolvableType; -import org.springframework.core.codec.Decoder; -import org.springframework.core.codec.Encoder; -import org.springframework.core.io.buffer.DataBuffer; -import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.core.convert.TypeDescriptor; import org.springframework.graphql.data.method.HandlerMethodArgumentResolver; import org.springframework.graphql.data.method.annotation.Argument; import org.springframework.graphql.data.method.annotation.ValueConstants; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpInputMessage; -import org.springframework.http.HttpOutputMessage; -import org.springframework.http.MediaType; -import org.springframework.http.converter.GenericHttpMessageConverter; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.MimeTypeUtils; import org.springframework.util.StringUtils; +import org.springframework.validation.DataBinder; /** * Resolver for {@link Argument @Argument} annotated method parameters, obtained @@ -51,37 +42,18 @@ import org.springframework.util.StringUtils; * declared type of the method parameter. * * @author Rossen Stoyanchev + * @author Brian Clozel * @since 1.0.0 */ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentResolver { - private final ArgumentConverter argumentConverter; - - /** - * Constructor with an - * {@link org.springframework.http.converter.HttpMessageConverter} to convert - * Map-based input arguments to higher level Objects. - */ - public ArgumentMethodArgumentResolver(GenericHttpMessageConverter converter) { - this.argumentConverter = new MessageConverterArgumentConverter(converter); - } - - /** - * Variant of - * {@link #ArgumentMethodArgumentResolver(GenericHttpMessageConverter)} - * to use an {@link Encoder} and {@link Decoder} to convert input arguments. - */ - public ArgumentMethodArgumentResolver(Decoder decoder, Encoder encoder) { - this.argumentConverter = new CodecArgumentConverter(decoder, encoder); - } - - @Override public boolean supportsParameter(MethodParameter parameter) { return parameter.getParameterAnnotation(Argument.class) != null; } @Override + @SuppressWarnings("unchecked") public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) throws Exception { Argument annotation = parameter.getParameterAnnotation(Argument.class); Assert.notNull(annotation, "No @Argument annotation"); @@ -99,136 +71,96 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso environment.getArgument(name) : environment.getArgumentOrDefault(name, annotation.defaultValue())); - Class parameterType = parameter.getParameterType(); + TypeDescriptor parameterType = new TypeDescriptor(parameter); if (rawValue == null) { if (annotation.required()) { throw new MissingArgumentException(name, parameter); } - if (parameterType.equals(Optional.class)) { + if (parameterType.getType().equals(Optional.class)) { return Optional.empty(); } return null; } - if (parameterType.isAssignableFrom(rawValue.getClass())) { - return returnValue(rawValue, parameterType); + 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; } - if (rawValue instanceof List) { - Assert.isAssignable(List.class, parameterType, - "Argument '" + name + "' is a List while the @Argument method parameter is " + parameterType); - List valueList = (List) rawValue; - Class elementType = parameter.nestedIfOptional().getNestedParameterType(); - if (valueList.isEmpty() || elementType.isAssignableFrom(valueList.get(0).getClass())) { - return returnValue(rawValue, parameterType); - } - } - - Object decodedValue = this.argumentConverter.convert(rawValue, parameter); - Assert.notNull(decodedValue, "Argument '" + name + "' with raw value '" + rawValue + "'was decoded to null"); - return returnValue(decodedValue, parameterType); + MethodParameter nestedParameter = parameter.nestedIfOptional(); + Object value = convert(rawValue, nestedParameter.getNestedParameterType()); + return returnValue(value, parameterType.getType()); } private Object returnValue(Object value, Class parameterType) { return (parameterType.equals(Optional.class) ? Optional.of(value) : value); } - - /** - * Contract to abstract use of an HttpMessageConverter vs Encoder/Decoder. - */ - private interface ArgumentConverter { - - @Nullable - Object convert(Object rawValue, MethodParameter targetParameter) throws Exception; - + @SuppressWarnings("unchecked") + private Object convert(Object rawValue, Class targetType) { + Object target; + if (rawValue instanceof Map) { + Constructor ctor = BeanUtils.getResolvableConstructor(targetType); + target = BeanUtils.instantiateClass(ctor); + DataBinder dataBinder = new DataBinder(target); + Assert.isTrue(ctor.getParameterCount() == 0, + () -> "Argument of type [" + targetType.getName() + + "] cannot be instantiated because of missing default constructor."); + MutablePropertyValues mpvs = extractPropertyValues((Map) rawValue); + dataBinder.bind(mpvs); + } + else if (targetType.isAssignableFrom(rawValue.getClass())) { + return returnValue(rawValue, targetType); + } + else { + DataBinder converter = new DataBinder(null); + target = converter.convertIfNecessary(rawValue, targetType); + Assert.isTrue(target != null, + () -> "Value of type [" + rawValue.getClass() + "] cannot be converted to argument of type [" + + targetType.getName() + "]."); + } + return target; } + private MutablePropertyValues extractPropertyValues(Map arguments) { + MutablePropertyValues mpvs = new MutablePropertyValues(); + Stack path = new Stack<>(); + visitArgumentMap(arguments, mpvs, path); + return mpvs; + } - /** - * HttpMessageConverter based implementation of ArgumentConverter. - */ - private static class MessageConverterArgumentConverter implements ArgumentConverter { - - private final GenericHttpMessageConverter converter; - - public MessageConverterArgumentConverter(GenericHttpMessageConverter converter) { - this.converter = converter; - } - - @Override - public Object convert(Object rawValue, MethodParameter targetParameter) throws IOException { - HttpOutputMessageAdapter outMessage = new HttpOutputMessageAdapter(); - this.converter.write(rawValue, MediaType.APPLICATION_JSON, outMessage); - - HttpInputMessageAdapter inMessage = new HttpInputMessageAdapter(outMessage); - return this.converter.read(targetParameter.getGenericParameterType(), rawValue.getClass(), inMessage); + @SuppressWarnings("unchecked") + private void visitArgumentMap(Map arguments, MutablePropertyValues mpvs, Stack path) { + for (String key : arguments.keySet()) { + path.push(key); + Object value = arguments.get(key); + if (value instanceof Map) { + visitArgumentMap((Map) value, mpvs, path); + } + else { + String propertyName = pathToPropertyName(path); + mpvs.add(propertyName, value); + } + path.pop(); } } - - /** - * Encoder/Decoder based implementation of ArgumentConverter. - */ - private static class CodecArgumentConverter implements ArgumentConverter { - - private final Decoder decoder; - - private final Encoder encoder; - - public CodecArgumentConverter(Decoder decoder, Encoder encoder) { - Assert.notNull(decoder, "Decoder is required"); - Assert.notNull(encoder, "Encoder is required"); - this.decoder = decoder; - this.encoder = encoder; + private String pathToPropertyName(Stack path) { + StringBuilder sb = new StringBuilder(); + Iterator it = path.iterator(); + while (it.hasNext()) { + sb.append(it.next()); + if (it.hasNext()) { + sb.append("."); + } } - - @Override - public Object convert(Object rawValue, MethodParameter targetParameter) { - DataBuffer dataBuffer = this.encoder.encodeValue( - rawValue, DefaultDataBufferFactory.sharedInstance, ResolvableType.forInstance(rawValue), - MimeTypeUtils.APPLICATION_JSON, Collections.emptyMap()); - - return this.decoder.decode( - dataBuffer, ResolvableType.forMethodParameter(targetParameter.nestedIfOptional()), - MimeTypeUtils.APPLICATION_JSON, Collections.emptyMap()); - } - } - - - private static class HttpInputMessageAdapter extends ByteArrayInputStream implements HttpInputMessage { - - HttpInputMessageAdapter(HttpOutputMessageAdapter messageAdapter) { - super(messageAdapter.toByteArray()); - } - - @Override - public InputStream getBody() { - return this; - } - - @Override - public HttpHeaders getHeaders() { - return HttpHeaders.EMPTY; - } - - } - - private static class HttpOutputMessageAdapter extends ByteArrayOutputStream implements HttpOutputMessage { - - private static final HttpHeaders noOpHeaders = new HttpHeaders(); - - @Override - public OutputStream getBody() { - return this; - } - - @Override - public HttpHeaders getHeaders() { - return noOpHeaders; - } - + return sb.toString(); } } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java new file mode 100644 index 00000000..8f7d46f3 --- /dev/null +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java @@ -0,0 +1,184 @@ +/* + * Copyright 2020-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.graphql.data.method.annotation.support; + + +import java.lang.reflect.Method; +import java.util.List; +import java.util.Map; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import graphql.schema.DataFetchingEnvironment; +import graphql.schema.DataFetchingEnvironmentImpl; +import org.junit.jupiter.api.Test; + +import org.springframework.core.DefaultParameterNameDiscoverer; +import org.springframework.core.MethodParameter; +import org.springframework.graphql.Book; +import org.springframework.graphql.data.method.annotation.Argument; +import org.springframework.graphql.data.method.annotation.MutationMapping; +import org.springframework.graphql.data.method.annotation.QueryMapping; +import org.springframework.stereotype.Controller; +import org.springframework.util.ClassUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link ArgumentMethodArgumentResolver}. + * + * @author Brian Clozel + */ +class ArgumentMethodArgumentResolverTests { + + private final ObjectMapper mapper = new ObjectMapper(); + + ArgumentMethodArgumentResolver resolver = new ArgumentMethodArgumentResolver(); + + @Test + void shouldSupportAnnotatedParameters() { + Method bookById = ClassUtils.getMethod(BookController.class, "bookById", Long.class); + MethodParameter methodParameter = getMethodParameter(bookById, 0); + assertThat(resolver.supportsParameter(methodParameter)).isTrue(); + } + + @Test + void shouldNotSupportParametersWithoutAnnotation() { + Method notSupported = ClassUtils.getMethod(BookController.class, "notSupported", String.class); + MethodParameter methodParameter = getMethodParameter(notSupported, 0); + assertThat(resolver.supportsParameter(methodParameter)).isFalse(); + } + + @Test + void shouldResolveBasicTypeArgument() throws Exception { + Method bookById = ClassUtils.getMethod(BookController.class, "bookById", Long.class); + String payload = "{\"id\": 42 }"; + DataFetchingEnvironment environment = initEnvironment(payload); + MethodParameter methodParameter = getMethodParameter(bookById, 0); + Object result = resolver.resolveArgument(methodParameter, environment); + assertThat(result).isNotNull().isInstanceOf(Long.class).isEqualTo(42L); + } + + @Test + void shouldResolveJavaBeanArgument() throws Exception { + Method addBook = ClassUtils.getMethod(BookController.class, "addBook", BookInput.class); + String payload = "{\"bookInput\": { \"name\": \"test name\", \"authorId\": 42} }"; + DataFetchingEnvironment environment = initEnvironment(payload); + MethodParameter methodParameter = getMethodParameter(addBook, 0); + Object result = resolver.resolveArgument(methodParameter, environment); + assertThat(result).isNotNull().isInstanceOf(BookInput.class); + assertThat((BookInput) result).hasFieldOrPropertyWithValue("name", "test name") + .hasFieldOrPropertyWithValue("authorId", 42L); + } + + @Test + void shouldResolveListOfJavaBeansArgument() throws Exception { + Method addBooks = ClassUtils.getMethod(BookController.class, "addBooks", List.class); + String payload = "{\"books\": [{ \"name\": \"first\", \"authorId\": 42}, { \"name\": \"second\", \"authorId\": 24}] }"; + DataFetchingEnvironment environment = initEnvironment(payload); + MethodParameter methodParameter = getMethodParameter(addBooks, 0); + Object result = resolver.resolveArgument(methodParameter, environment); + assertThat(result).isNotNull().isInstanceOf(List.class); + assertThat(result).asList().allMatch(item -> item instanceof Book) + .extracting("name").containsExactly("first", "second"); + } + + @Test + void shouldResolveNestedJavaBeanArgument() throws Exception { + Method addBookWithNestedAuthor = ClassUtils.getMethod(BookController.class, "addBookWithNestedAuthor", Book.class); + String payload = "{\"book\": { \"name\": \"test name\", \"author\": { \"firstName\": \"Jane\", \"lastName\": \"Spring\"} } }"; + DataFetchingEnvironment environment = initEnvironment(payload); + MethodParameter methodParameter = getMethodParameter(addBookWithNestedAuthor, 0); + Object result = resolver.resolveArgument(methodParameter, environment); + assertThat(result).isNotNull().isInstanceOf(Book.class); + assertThat((Book) result).hasFieldOrPropertyWithValue("name", "test name"); + assertThat(((Book) result).getAuthor()).isNotNull() + .hasFieldOrPropertyWithValue("firstName", "Jane") + .hasFieldOrPropertyWithValue("lastName", "Spring"); + } + + private MethodParameter getMethodParameter(Method method, int index) { + MethodParameter methodParameter = new MethodParameter(method, index); + methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer()); + return methodParameter; + } + + private DataFetchingEnvironment initEnvironment(String jsonPayload) throws JsonProcessingException { + Map arguments = mapper.readValue(jsonPayload, new TypeReference>() { + }); + return DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build(); + } + + // BeanWrapper + // ModelAttributeProcessor for constructor based instantiation + + // look at DGS PR for this issue + + + @Controller + static class BookController { + + public void notSupported(String param) { + + } + + @QueryMapping + public Book bookById(@Argument Long id) { + return null; + } + + @MutationMapping + public Book addBook(@Argument BookInput bookInput) { + return null; + } + + @MutationMapping + public List addBooks(@Argument List books) { + return null; + } + + @MutationMapping + public Book addBookWithNestedAuthor(@Argument Book book) { + return null; + } + } + + static class BookInput { + + String name; + + Long authorId; + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + + public Long getAuthorId() { + return this.authorId; + } + + public void setAuthorId(Long authorId) { + this.authorId = authorId; + } + } + +} \ No newline at end of file