Refactor input argument binding
Prior to this commit, `@Argument`-annotated controller method parameters were bound from the environment by serializing the environment argument map to JSON and deserialized back to the target parameter type. This allowed to cover a large spectrum of cases but showed some limitations: performance could be improved and custom scalars could be overridden in the process. This commit removes the (de)serialization process and instead: * runs through the environment argument map to collect all properties in a `MutablePropertyValues` map * instantiate the target type if necessary * binds properties to the target type using a `DataBinder` With this change, scalar types should not be erased and arguments now support a wider range of Collection types. Closes gh-122
This commit is contained in:
Brian Clozel
@@ -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");
|
||||
|
||||
@@ -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<Object> 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<Object> decoder, Encoder<Object> 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<Object> rawCollection = (Collection<Object>) rawValue;
|
||||
Collection<Object> 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<String, Object> arguments) {
|
||||
MutablePropertyValues mpvs = new MutablePropertyValues();
|
||||
Stack<String> path = new Stack<>();
|
||||
visitArgumentMap(arguments, mpvs, path);
|
||||
return mpvs;
|
||||
}
|
||||
|
||||
/**
|
||||
* HttpMessageConverter based implementation of ArgumentConverter.
|
||||
*/
|
||||
private static class MessageConverterArgumentConverter implements ArgumentConverter {
|
||||
|
||||
private final GenericHttpMessageConverter<Object> converter;
|
||||
|
||||
public MessageConverterArgumentConverter(GenericHttpMessageConverter<Object> 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<String, Object> arguments, MutablePropertyValues mpvs, Stack<String> path) {
|
||||
for (String key : arguments.keySet()) {
|
||||
path.push(key);
|
||||
Object value = arguments.get(key);
|
||||
if (value instanceof Map) {
|
||||
visitArgumentMap((Map<String, Object>) 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<Object> decoder;
|
||||
|
||||
private final Encoder<Object> encoder;
|
||||
|
||||
public CodecArgumentConverter(Decoder<Object> decoder, Encoder<Object> encoder) {
|
||||
Assert.notNull(decoder, "Decoder is required");
|
||||
Assert.notNull(encoder, "Encoder is required");
|
||||
this.decoder = decoder;
|
||||
this.encoder = encoder;
|
||||
private String pathToPropertyName(Stack<String> path) {
|
||||
StringBuilder sb = new StringBuilder();
|
||||
Iterator<String> 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();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -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<String, Object> arguments = mapper.readValue(jsonPayload, new TypeReference<Map<String, Object>>() {
|
||||
});
|
||||
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<Book> addBooks(@Argument List<Book> 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;
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user